mirror of
https://github.com/discourse/discourse.git
synced 2024-11-22 11:23:25 +08:00
DEV: moves logic from job to a service (#22691)
`Jobs::AutoJoinChannelBatch` was holding a lot of logic which should be in a service. Moreover, this refactoring is the opportunity to address a bug which could cause a duplicate key error. From now when trying to insert a new membership it won't fail if a membership is already present. Example error: ``` Job exception: ERROR: duplicate key value violates unique constraint "user_chat_channel_unique_memberships" DETAIL: Key (user_id, chat_channel_id)=(1, 2) already exists. Backtrace rack-mini-profiler-3.1.0/lib/patches/db/pg.rb:110:in `exec' rack-mini-profiler-3.1.0/lib/patches/db/pg.rb:110:in `async_exec' (eval):29:in `async_exec' mini_sql-1.4.0/lib/mini_sql/postgres/connection.rb:209:in `run' mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:38:in `block in run' mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:34:in `block in with_lock' activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt' activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize' activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt' activesupport-7.0.5.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize' mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:34:in `with_lock' mini_sql-1.4.0/lib/mini_sql/active_record_postgres/connection.rb:38:in `run' mini_sql-1.4.0/lib/mini_sql/postgres/connection.rb:64:in `query_single' /var/www/discourse/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb:38:in `execute' ``` Note this commit is also using main branch of `shoulda-matchers` as the gem has not been released yet. Co-authored-by: Loïc Guitaut <5648+Flink@users.noreply.github.com>
This commit is contained in:
parent
2d567cee26
commit
05aa55e172
2
Gemfile
2
Gemfile
|
@ -158,7 +158,7 @@ group :test, :development do
|
|||
|
||||
gem "rspec-rails"
|
||||
|
||||
gem "shoulda-matchers", require: false
|
||||
gem "shoulda-matchers", require: false, github: "thoughtbot/shoulda-matchers"
|
||||
gem "rspec-html-matchers"
|
||||
gem "byebug", require: ENV["RM_INFO"].nil?, platform: :mri
|
||||
gem "rubocop-discourse", require: false
|
||||
|
|
11
Gemfile.lock
11
Gemfile.lock
|
@ -7,6 +7,13 @@ GIT
|
|||
concurrent-ruby (~> 1.0)
|
||||
rack (> 1, < 3)
|
||||
|
||||
GIT
|
||||
remote: https://github.com/thoughtbot/shoulda-matchers.git
|
||||
revision: 783a90554053002017510285bc736099b2749c22
|
||||
specs:
|
||||
shoulda-matchers (5.3.0)
|
||||
activesupport (>= 5.2.0)
|
||||
|
||||
GEM
|
||||
remote: https://rubygems.org/
|
||||
specs:
|
||||
|
@ -460,8 +467,6 @@ GEM
|
|||
rexml (~> 3.2, >= 3.2.5)
|
||||
rubyzip (>= 1.2.2, < 3.0)
|
||||
websocket (~> 1.0)
|
||||
shoulda-matchers (5.3.0)
|
||||
activesupport (>= 5.2.0)
|
||||
sidekiq (6.5.9)
|
||||
connection_pool (>= 2.2.5, < 3)
|
||||
rack (~> 2.0)
|
||||
|
@ -651,7 +656,7 @@ DEPENDENCIES
|
|||
rubyzip
|
||||
sanitize
|
||||
selenium-webdriver
|
||||
shoulda-matchers
|
||||
shoulda-matchers!
|
||||
sidekiq
|
||||
simplecov
|
||||
sprockets!
|
||||
|
|
|
@ -1,79 +0,0 @@
|
|||
# NOTE: When changing auto-join logic, make sure to update the `settings.auto_join_users_info` translation as well.
|
||||
# frozen_string_literal: true
|
||||
|
||||
module Jobs
|
||||
class AutoJoinChannelBatch < ::Jobs::Base
|
||||
def execute(args)
|
||||
return "starts_at or ends_at missing" if args[:starts_at].blank? || args[:ends_at].blank?
|
||||
start_user_id = args[:starts_at].to_i
|
||||
end_user_id = args[:ends_at].to_i
|
||||
|
||||
return "End is higher than start" if end_user_id < start_user_id
|
||||
|
||||
channel =
|
||||
ChatChannel.find_by(
|
||||
id: args[:chat_channel_id],
|
||||
auto_join_users: true,
|
||||
chatable_type: "Category",
|
||||
)
|
||||
|
||||
return if !channel
|
||||
|
||||
category = channel.chatable
|
||||
return if !category
|
||||
|
||||
query_args = {
|
||||
chat_channel_id: channel.id,
|
||||
start: start_user_id,
|
||||
end: end_user_id,
|
||||
suspended_until: Time.zone.now,
|
||||
last_seen_at: 3.months.ago,
|
||||
channel_category: channel.chatable_id,
|
||||
permission_type: CategoryGroup.permission_types[:create_post],
|
||||
everyone: Group::AUTO_GROUPS[:everyone],
|
||||
mode: Chat::UserChatChannelMembership.join_modes[:automatic],
|
||||
}
|
||||
|
||||
new_member_ids = DB.query_single(create_memberships_query(category), query_args)
|
||||
# Only do this if we are running auto-join for a single user, if we
|
||||
# are doing it for many then we should do it after all batches are
|
||||
# complete for the channel in Jobs::AutoJoinChannelMemberships
|
||||
if start_user_id == end_user_id
|
||||
Chat::ChatChannelMembershipManager.new(channel).recalculate_user_count
|
||||
end
|
||||
|
||||
Chat::Publisher.publish_new_channel(channel.reload, User.where(id: new_member_ids))
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def create_memberships_query(category)
|
||||
query = <<~SQL
|
||||
INSERT INTO user_chat_channel_memberships (user_id, chat_channel_id, following, created_at, updated_at, join_mode)
|
||||
SELECT DISTINCT(users.id), :chat_channel_id, TRUE, NOW(), NOW(), :mode
|
||||
FROM users
|
||||
INNER JOIN user_options uo ON uo.user_id = users.id
|
||||
LEFT OUTER JOIN user_chat_channel_memberships uccm ON
|
||||
uccm.chat_channel_id = :chat_channel_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 <= :permission_type
|
||||
|
||||
WHERE (users.id >= :start AND users.id <= :end) AND
|
||||
users.staged IS FALSE AND
|
||||
users.active AND
|
||||
NOT EXISTS(SELECT 1 FROM anonymous_users a WHERE a.user_id = users.id) AND
|
||||
(suspended_till IS NULL OR suspended_till <= :suspended_until) AND
|
||||
(last_seen_at IS NULL OR last_seen_at > :last_seen_at) AND
|
||||
uo.chat_enabled AND
|
||||
uccm.id IS NULL AND
|
||||
(NOT EXISTS(SELECT 1 FROM category_groups WHERE category_id = :channel_category)
|
||||
OR EXISTS (SELECT 1 FROM category_groups WHERE category_id = :channel_category AND group_id = :everyone AND permission_type <= :permission_type)
|
||||
OR cg.category_id = :channel_category)
|
||||
|
||||
RETURNING user_chat_channel_memberships.user_id
|
||||
SQL
|
||||
end
|
||||
end
|
||||
end
|
|
@ -1,81 +1,17 @@
|
|||
# NOTE: When changing auto-join logic, make sure to update the `settings.auto_join_users_info` translation as well.
|
||||
# frozen_string_literal: true
|
||||
|
||||
module Jobs
|
||||
module Chat
|
||||
class AutoJoinChannelBatch < ::Jobs::Base
|
||||
class AutoJoinChannelBatch < ServiceJob
|
||||
def execute(args)
|
||||
return "starts_at or ends_at missing" if args[:starts_at].blank? || args[:ends_at].blank?
|
||||
start_user_id = args[:starts_at].to_i
|
||||
end_user_id = args[:ends_at].to_i
|
||||
|
||||
return "End is higher than start" if end_user_id < start_user_id
|
||||
|
||||
channel =
|
||||
::Chat::Channel.find_by(
|
||||
id: args[:chat_channel_id],
|
||||
auto_join_users: true,
|
||||
chatable_type: "Category",
|
||||
)
|
||||
|
||||
return if !channel
|
||||
|
||||
category = channel.chatable
|
||||
return if !category
|
||||
|
||||
query_args = {
|
||||
chat_channel_id: channel.id,
|
||||
start: start_user_id,
|
||||
end: end_user_id,
|
||||
suspended_until: Time.zone.now,
|
||||
last_seen_at: 3.months.ago,
|
||||
channel_category: channel.chatable_id,
|
||||
permission_type: CategoryGroup.permission_types[:create_post],
|
||||
everyone: Group::AUTO_GROUPS[:everyone],
|
||||
mode: ::Chat::UserChatChannelMembership.join_modes[:automatic],
|
||||
}
|
||||
|
||||
new_member_ids = DB.query_single(create_memberships_query(category), query_args)
|
||||
|
||||
# Only do this if we are running auto-join for a single user, if we
|
||||
# are doing it for many then we should do it after all batches are
|
||||
# complete for the channel in Jobs::Chat::AutoJoinChannelMemberships
|
||||
if start_user_id == end_user_id
|
||||
::Chat::ChannelMembershipManager.new(channel).recalculate_user_count
|
||||
with_service(::Chat::AutoJoinChannelBatch, **args) do
|
||||
on_failed_contract do |contract|
|
||||
Rails.logger.error(contract.errors.full_messages.join(", "))
|
||||
end
|
||||
on_model_not_found(:channel) do
|
||||
Rails.logger.error("Channel not found (id=#{result.contract.channel_id})")
|
||||
end
|
||||
end
|
||||
|
||||
::Chat::Publisher.publish_new_channel(channel.reload, User.where(id: new_member_ids))
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def create_memberships_query(category)
|
||||
query = <<~SQL
|
||||
INSERT INTO user_chat_channel_memberships (user_id, chat_channel_id, following, created_at, updated_at, join_mode)
|
||||
SELECT DISTINCT(users.id), :chat_channel_id, TRUE, NOW(), NOW(), :mode
|
||||
FROM users
|
||||
INNER JOIN user_options uo ON uo.user_id = users.id
|
||||
LEFT OUTER JOIN user_chat_channel_memberships uccm ON
|
||||
uccm.chat_channel_id = :chat_channel_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 <= :permission_type
|
||||
|
||||
WHERE (users.id >= :start AND users.id <= :end) AND
|
||||
users.staged IS FALSE AND
|
||||
users.active AND
|
||||
NOT EXISTS(SELECT 1 FROM anonymous_users a WHERE a.user_id = users.id) AND
|
||||
(suspended_till IS NULL OR suspended_till <= :suspended_until) AND
|
||||
(last_seen_at IS NULL OR last_seen_at > :last_seen_at) AND
|
||||
uo.chat_enabled AND
|
||||
uccm.id IS NULL AND
|
||||
(NOT EXISTS(SELECT 1 FROM category_groups WHERE category_id = :channel_category)
|
||||
OR EXISTS (SELECT 1 FROM category_groups WHERE category_id = :channel_category AND group_id = :everyone AND permission_type <= :permission_type)
|
||||
OR cg.category_id = :channel_category)
|
||||
|
||||
RETURNING user_chat_channel_memberships.user_id
|
||||
SQL
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,49 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
module Chat
|
||||
module Action
|
||||
class CreateMembershipsForAutoJoin
|
||||
def self.call(channel:, contract:)
|
||||
query_args = {
|
||||
chat_channel_id: channel.id,
|
||||
start: contract.start_user_id,
|
||||
end: contract.end_user_id,
|
||||
suspended_until: Time.zone.now,
|
||||
last_seen_at: 3.months.ago,
|
||||
channel_category: channel.category.id,
|
||||
permission_type: CategoryGroup.permission_types[:create_post],
|
||||
everyone: Group::AUTO_GROUPS[:everyone],
|
||||
mode: ::Chat::UserChatChannelMembership.join_modes[:automatic],
|
||||
}
|
||||
::DB.query_single(<<~SQL, query_args)
|
||||
INSERT INTO user_chat_channel_memberships (user_id, chat_channel_id, following, created_at, updated_at, join_mode)
|
||||
SELECT DISTINCT(users.id), :chat_channel_id, TRUE, NOW(), NOW(), :mode
|
||||
FROM users
|
||||
INNER JOIN user_options uo ON uo.user_id = users.id
|
||||
LEFT OUTER JOIN user_chat_channel_memberships uccm ON
|
||||
uccm.chat_channel_id = :chat_channel_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 <= :permission_type
|
||||
|
||||
WHERE (users.id >= :start AND users.id <= :end) AND
|
||||
users.staged IS FALSE AND
|
||||
users.active AND
|
||||
NOT EXISTS(SELECT 1 FROM anonymous_users a WHERE a.user_id = users.id) AND
|
||||
(suspended_till IS NULL OR suspended_till <= :suspended_until) AND
|
||||
(last_seen_at IS NULL OR last_seen_at > :last_seen_at) AND
|
||||
uo.chat_enabled AND
|
||||
|
||||
(NOT EXISTS(SELECT 1 FROM category_groups WHERE category_id = :channel_category)
|
||||
OR EXISTS (SELECT 1 FROM category_groups WHERE category_id = :channel_category AND group_id = :everyone AND permission_type <= :permission_type)
|
||||
OR cg.category_id = :channel_category)
|
||||
|
||||
ON CONFLICT DO NOTHING
|
||||
|
||||
RETURNING user_chat_channel_memberships.user_id
|
||||
SQL
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
68
plugins/chat/app/services/chat/auto_join_channel_batch.rb
Normal file
68
plugins/chat/app/services/chat/auto_join_channel_batch.rb
Normal file
|
@ -0,0 +1,68 @@
|
|||
# NOTE: When changing auto-join logic, make sure to update the `settings.auto_join_users_info` translation as well.
|
||||
# frozen_string_literal: true
|
||||
|
||||
module Chat
|
||||
# Service responsible to create memberships for a channel and a section of user ids
|
||||
#
|
||||
# @example
|
||||
# Chat::AutoJoinChannelBatch.call(
|
||||
# channel_id: 1,
|
||||
# start_user_id: 27,
|
||||
# end_user_id: 58,
|
||||
# )
|
||||
#
|
||||
class AutoJoinChannelBatch
|
||||
include Service::Base
|
||||
|
||||
contract
|
||||
model :channel
|
||||
step :create_memberships
|
||||
step :recalculate_user_count
|
||||
step :publish_new_channel
|
||||
|
||||
class Contract
|
||||
# Backward-compatible attributes
|
||||
attribute :chat_channel_id, :integer
|
||||
attribute :starts_at, :integer
|
||||
attribute :ends_at, :integer
|
||||
|
||||
# New attributes
|
||||
attribute :channel_id, :integer
|
||||
attribute :start_user_id, :integer
|
||||
attribute :end_user_id, :integer
|
||||
|
||||
validates :channel_id, :start_user_id, :end_user_id, presence: true
|
||||
validates :end_user_id, comparison: { greater_than_or_equal_to: :start_user_id }
|
||||
|
||||
# TODO (joffrey): remove after migration is done
|
||||
before_validation do
|
||||
self.channel_id ||= chat_channel_id
|
||||
self.start_user_id ||= starts_at
|
||||
self.end_user_id ||= ends_at
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def fetch_channel(contract:, **)
|
||||
::Chat::CategoryChannel.find_by(id: contract.channel_id, auto_join_users: true)
|
||||
end
|
||||
|
||||
def create_memberships(channel:, contract:, **)
|
||||
context.added_user_ids =
|
||||
::Chat::Action::CreateMembershipsForAutoJoin.call(channel: channel, contract: contract)
|
||||
end
|
||||
|
||||
def recalculate_user_count(channel:, added_user_ids:, **)
|
||||
# Only do this if we are running auto-join for a single user, if we
|
||||
# are doing it for many then we should do it after all batches are
|
||||
# complete for the channel in Jobs::AutoJoinChannelMemberships
|
||||
return unless added_user_ids.one?
|
||||
::Chat::ChannelMembershipManager.new(channel).recalculate_user_count
|
||||
end
|
||||
|
||||
def publish_new_channel(channel:, added_user_ids:, **)
|
||||
::Chat::Publisher.publish_new_channel(channel.reload, User.where(id: added_user_ids))
|
||||
end
|
||||
end
|
||||
end
|
|
@ -3,251 +3,34 @@
|
|||
require "rails_helper"
|
||||
|
||||
describe Jobs::Chat::AutoJoinChannelBatch do
|
||||
subject(:job) { described_class.new }
|
||||
|
||||
describe "#execute" do
|
||||
fab!(:category) { Fabricate(:category) }
|
||||
let!(:user) { Fabricate(:user, last_seen_at: 15.minutes.ago) }
|
||||
let(:channel) { Fabricate(:chat_channel, auto_join_users: true, chatable: category) }
|
||||
|
||||
it "joins all valid users in the batch" do
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
end
|
||||
|
||||
it "doesn't join users outside the batch" do
|
||||
another_user = Fabricate(:user, last_seen_at: 15.minutes.ago)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
assert_user_skipped(channel, another_user)
|
||||
end
|
||||
|
||||
it "doesn't join suspended users" do
|
||||
user.update!(suspended_till: 1.year.from_now)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_user_skipped(channel, user)
|
||||
end
|
||||
|
||||
it "doesn't join users last_seen more than 3 months ago" do
|
||||
user.update!(last_seen_at: 4.months.ago)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_user_skipped(channel, user)
|
||||
end
|
||||
|
||||
it "joins users with last_seen set to null" do
|
||||
user.update!(last_seen_at: nil)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
end
|
||||
|
||||
it "does nothing if the channel is invalid" do
|
||||
job.execute(chat_channel_id: -1, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_user_skipped(channel, user)
|
||||
end
|
||||
|
||||
it "does nothing if the channel chatable is not a category" do
|
||||
direct_message = Fabricate(:direct_message)
|
||||
channel.update!(chatable: direct_message)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_user_skipped(channel, user)
|
||||
end
|
||||
|
||||
it "enqueues the user count update job and marks the channel user count as stale" do
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
expect_job_enqueued(
|
||||
job: Jobs::Chat::UpdateChannelUserCount,
|
||||
args: {
|
||||
chat_channel_id: channel.id,
|
||||
},
|
||||
it "can successfully queue this job" do
|
||||
expect {
|
||||
Jobs.enqueue(
|
||||
described_class,
|
||||
channel_id: Fabricate(:chat_channel).id,
|
||||
start_user_id: 0,
|
||||
end_user_id: 10,
|
||||
)
|
||||
}.to change(Jobs::Chat::AutoJoinChannelBatch.jobs, :size).by(1)
|
||||
end
|
||||
|
||||
expect(channel.reload.user_count_stale).to eq(true)
|
||||
end
|
||||
context "when contract fails" do
|
||||
before { Jobs.run_immediately! }
|
||||
|
||||
it "does not enqueue the user count update job or mark the channel user count as stale when there is more than use user" do
|
||||
user_2 = Fabricate(:user)
|
||||
expect_not_enqueued_with(
|
||||
job: Jobs::Chat::UpdateChannelUserCount,
|
||||
args: {
|
||||
chat_channel_id: channel.id,
|
||||
},
|
||||
) { job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user_2.id) }
|
||||
it "logs an error" do
|
||||
Rails.logger.expects(:error).with(regexp_matches(/Channel can't be blank/)).at_least_once
|
||||
|
||||
expect(channel.reload.user_count_stale).to eq(false)
|
||||
end
|
||||
|
||||
it "ignores users without chat_enabled" do
|
||||
user.user_option.update!(chat_enabled: false)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_user_skipped(channel, user)
|
||||
end
|
||||
|
||||
it "sets the join reason to automatic" do
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
new_membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
|
||||
expect(new_membership.automatic?).to eq(true)
|
||||
end
|
||||
|
||||
it "skips anonymous users" do
|
||||
user_2 = Fabricate(:anonymous)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user_2.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
assert_user_skipped(channel, user_2)
|
||||
end
|
||||
|
||||
it "skips non-active users" do
|
||||
user_2 = Fabricate(:user, active: false, last_seen_at: 15.minutes.ago)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user_2.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
assert_user_skipped(channel, user_2)
|
||||
end
|
||||
|
||||
it "skips staged users" do
|
||||
user_2 = Fabricate(:user, staged: true, last_seen_at: 15.minutes.ago)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user_2.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
assert_user_skipped(channel, user_2)
|
||||
end
|
||||
|
||||
it "adds every user in the batch" do
|
||||
user_2 = Fabricate(:user, last_seen_at: 15.minutes.ago)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user_2.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user, user_2])
|
||||
end
|
||||
|
||||
it "publishes a message only to joined users" do
|
||||
messages =
|
||||
MessageBus.track_publish("/chat/new-channel") do
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
end
|
||||
|
||||
expect(messages.size).to eq(1)
|
||||
expect(messages.first.data.dig(:channel, :id)).to eq(channel.id)
|
||||
end
|
||||
|
||||
describe "context when the channel's category is read restricted" do
|
||||
fab!(:chatters_group) { Fabricate(:group) }
|
||||
let(:private_category) { Fabricate(:private_category, group: chatters_group) }
|
||||
let(:channel) { Fabricate(:chat_channel, auto_join_users: true, chatable: private_category) }
|
||||
|
||||
before { chatters_group.add(user) }
|
||||
|
||||
it "only joins group members with access to the category" do
|
||||
another_user = Fabricate(:user, last_seen_at: 15.minutes.ago)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: another_user.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
assert_user_skipped(channel, another_user)
|
||||
end
|
||||
|
||||
it "works if the user has access through more than one group" do
|
||||
second_chatters_group = Fabricate(:group)
|
||||
Fabricate(:category_group, category: category, group: second_chatters_group)
|
||||
second_chatters_group.add(user)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: user.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user])
|
||||
end
|
||||
|
||||
it "joins every user with access to the category" do
|
||||
another_user = Fabricate(:user, last_seen_at: 15.minutes.ago)
|
||||
chatters_group.add(another_user)
|
||||
|
||||
job.execute(chat_channel_id: channel.id, starts_at: user.id, ends_at: another_user.id)
|
||||
|
||||
assert_users_follows_channel(channel, [user, another_user])
|
||||
end
|
||||
|
||||
it "doesn't join users with read-only access to the category" do
|
||||
restricted_category = Fabricate(:category, read_restricted: true)
|
||||
another_user = Fabricate(:user, last_seen_at: 15.minutes.ago)
|
||||
non_chatters_group = Fabricate(:group)
|
||||
readonly_channel =
|
||||
Fabricate(:category_channel, chatable: restricted_category, auto_join_users: true)
|
||||
Fabricate(
|
||||
:category_group,
|
||||
category: restricted_category,
|
||||
group: non_chatters_group,
|
||||
permission_type: CategoryGroup.permission_types[:readonly],
|
||||
)
|
||||
non_chatters_group.add(another_user)
|
||||
|
||||
job.execute(
|
||||
chat_channel_id: readonly_channel.id,
|
||||
starts_at: another_user.id,
|
||||
ends_at: another_user.id,
|
||||
)
|
||||
|
||||
assert_user_skipped(readonly_channel, another_user)
|
||||
end
|
||||
|
||||
it "does join users with at least one group with create_post or full permission" do
|
||||
restricted_category = Fabricate(:category, read_restricted: true)
|
||||
another_user = Fabricate(:user, last_seen_at: 15.minutes.ago)
|
||||
non_chatters_group = Fabricate(:group)
|
||||
private_channel =
|
||||
Fabricate(:category_channel, chatable: restricted_category, auto_join_users: true)
|
||||
Fabricate(
|
||||
:category_group,
|
||||
category: restricted_category,
|
||||
group: non_chatters_group,
|
||||
permission_type: CategoryGroup.permission_types[:readonly],
|
||||
)
|
||||
non_chatters_group.add(another_user)
|
||||
|
||||
other_group = Fabricate(:group)
|
||||
Fabricate(
|
||||
:category_group,
|
||||
category: restricted_category,
|
||||
group: other_group,
|
||||
permission_type: CategoryGroup.permission_types[:create_post],
|
||||
)
|
||||
other_group.add(another_user)
|
||||
|
||||
job.execute(
|
||||
chat_channel_id: private_channel.id,
|
||||
starts_at: another_user.id,
|
||||
ends_at: another_user.id,
|
||||
)
|
||||
|
||||
assert_users_follows_channel(private_channel, [another_user])
|
||||
end
|
||||
Jobs.enqueue(described_class)
|
||||
end
|
||||
end
|
||||
|
||||
def assert_users_follows_channel(channel, users)
|
||||
new_memberships = Chat::UserChatChannelMembership.where(user: users, chat_channel: channel)
|
||||
expect(new_memberships.length).to eq(users.length)
|
||||
expect(new_memberships.all?(&:following)).to eq(true)
|
||||
end
|
||||
context "when model is not found" do
|
||||
before { Jobs.run_immediately! }
|
||||
|
||||
def assert_user_skipped(channel, user)
|
||||
new_membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel)
|
||||
expect(new_membership).to be_nil
|
||||
it "logs an error" do
|
||||
Rails.logger.expects(:error).with("Channel not found (id=-999)").at_least_once
|
||||
|
||||
Jobs.enqueue(described_class, channel_id: -999, start_user_id: 1, end_user_id: 2)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,172 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
RSpec.describe Chat::Action::CreateMembershipsForAutoJoin do
|
||||
subject(:action) { described_class.call(channel: channel, contract: contract) }
|
||||
|
||||
fab!(:channel) { Fabricate(:chat_channel, auto_join_users: true) }
|
||||
fab!(:user_1) { Fabricate(:user, last_seen_at: 15.minutes.ago) }
|
||||
|
||||
let(:start_user_id) { user_1.id }
|
||||
let(:end_user_id) { user_1.id }
|
||||
let(:contract) { OpenStruct.new(start_user_id: start_user_id, end_user_id: end_user_id) }
|
||||
|
||||
it "adds correct members" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
|
||||
it "sets the reason to automatic" do
|
||||
action
|
||||
expect(channel.membership_for(user_1)).to be_automatic
|
||||
end
|
||||
|
||||
context "with others users not in the batch" do
|
||||
fab!(:user_2) { Fabricate(:user) }
|
||||
|
||||
it "adds correct members" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
end
|
||||
|
||||
context "with suspended users" do
|
||||
before { user_1.update!(suspended_till: 1.year.from_now) }
|
||||
|
||||
it "skips suspended users" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "with users not seen recently" do
|
||||
before { user_1.update!(last_seen_at: 4.months.ago) }
|
||||
|
||||
it "skips users last_seen more than 3 months ago" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "with never seen users" do
|
||||
before { user_1.update!(last_seen_at: nil) }
|
||||
|
||||
it "includes users with last_seen set to null" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
end
|
||||
|
||||
context "with disabled chat users" do
|
||||
before { user_1.user_option.update!(chat_enabled: false) }
|
||||
|
||||
it "skips users without chat_enabled" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "with anonymous users" do
|
||||
fab!(:user_1) { Fabricate(:anonymous, last_seen_at: 15.minutes.ago) }
|
||||
|
||||
it "skips anonymous users" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "with inactive users" do
|
||||
before { user_1.update!(active: false) }
|
||||
|
||||
it "skips inactive users" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "with staged users" do
|
||||
before { user_1.update!(staged: true) }
|
||||
|
||||
it "skips staged users" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "when user is already a member" do
|
||||
before { channel.add(user_1) }
|
||||
|
||||
it "is a noop" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "when category is restricted" do
|
||||
fab!(:user_1) { Fabricate(:user) }
|
||||
fab!(:user_2) { Fabricate(:user) }
|
||||
fab!(:group_1) { Fabricate(:group) }
|
||||
fab!(:channel) { Fabricate(:private_category_channel, group: group_1, auto_join_users: true) }
|
||||
|
||||
let(:end_user_id) { user_2.id }
|
||||
|
||||
before { group_1.add(user_1) }
|
||||
|
||||
it "only joins users with access to the category through the group" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
|
||||
context "when the user has access through multiple groups" do
|
||||
fab!(:group_2) { Fabricate(:group) }
|
||||
|
||||
before do
|
||||
channel.category.category_groups.create!(
|
||||
group_id: group_2.id,
|
||||
permission_type: CategoryGroup.permission_types[:full],
|
||||
)
|
||||
group_2.add(user_1)
|
||||
end
|
||||
|
||||
it "correctly joins the user" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
end
|
||||
|
||||
context "when the category group is read only" do
|
||||
fab!(:channel) { Fabricate(:private_category_channel, auto_join_users: true) }
|
||||
|
||||
before do
|
||||
channel.category.category_groups.create!(
|
||||
group_id: group_1.id,
|
||||
permission_type: CategoryGroup.permission_types[:readonly],
|
||||
)
|
||||
group_1.add(user_1)
|
||||
end
|
||||
|
||||
it "doesn’t join the users of the group" do
|
||||
expect(action).to eq([])
|
||||
end
|
||||
end
|
||||
|
||||
context "when the category group has create post permission" do
|
||||
fab!(:channel) { Fabricate(:private_category_channel, auto_join_users: true) }
|
||||
|
||||
before do
|
||||
channel.category.category_groups.create!(
|
||||
group_id: group_1.id,
|
||||
permission_type: CategoryGroup.permission_types[:create_post],
|
||||
)
|
||||
group_1.add(user_1)
|
||||
end
|
||||
|
||||
it "correctly joins the user" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
end
|
||||
|
||||
context "when user has allowed groups and disallowed groups" do
|
||||
fab!(:group_2) { Fabricate(:group) }
|
||||
|
||||
before do
|
||||
channel.category.category_groups.create!(
|
||||
group_id: group_2.id,
|
||||
permission_type: CategoryGroup.permission_types[:readonly],
|
||||
)
|
||||
group_2.add(user_1)
|
||||
end
|
||||
|
||||
it "correctly joins the user" do
|
||||
expect(action).to eq([user_1.id])
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
130
plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb
Normal file
130
plugins/chat/spec/services/chat/auto_join_channel_batch_spec.rb
Normal file
|
@ -0,0 +1,130 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
|
||||
describe Chat::AutoJoinChannelBatch do
|
||||
describe Chat::AutoJoinChannelBatch::Contract, type: :model do
|
||||
subject(:contract) { described_class.new(start_user_id: 10) }
|
||||
|
||||
it { is_expected.to validate_presence_of(:channel_id) }
|
||||
it { is_expected.to validate_presence_of(:start_user_id) }
|
||||
it { is_expected.to validate_presence_of(:end_user_id) }
|
||||
it do
|
||||
is_expected.to validate_comparison_of(:end_user_id).is_greater_than_or_equal_to(
|
||||
:start_user_id,
|
||||
)
|
||||
end
|
||||
|
||||
describe "Backward compatibility" do
|
||||
subject(:contract) { described_class.new(args) }
|
||||
|
||||
before { contract.valid? }
|
||||
|
||||
context "when providing 'chat_channel_id'" do
|
||||
let(:args) { { chat_channel_id: 2 } }
|
||||
|
||||
it "sets 'channel_id'" do
|
||||
expect(contract.channel_id).to eq(2)
|
||||
end
|
||||
end
|
||||
|
||||
context "when providing 'starts_at'" do
|
||||
let(:args) { { starts_at: 5 } }
|
||||
|
||||
it "sets 'start_user_id'" do
|
||||
expect(contract.start_user_id).to eq(5)
|
||||
end
|
||||
end
|
||||
|
||||
context "when providing 'ends_at'" do
|
||||
let(:args) { { ends_at: 8 } }
|
||||
|
||||
it "sets 'end_user_id'" do
|
||||
expect(contract.end_user_id).to eq(8)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe ".call" do
|
||||
subject(:result) { described_class.call(params) }
|
||||
|
||||
fab!(:channel) { Fabricate(:chat_channel, auto_join_users: true) }
|
||||
|
||||
let(:channel_id) { channel.id }
|
||||
let(:start_user_id) { 0 }
|
||||
let(:end_user_id) { 10 }
|
||||
let(:params) do
|
||||
{ channel_id: channel_id, start_user_id: start_user_id, end_user_id: end_user_id }
|
||||
end
|
||||
|
||||
context "when arguments are invalid" do
|
||||
let(:channel_id) { nil }
|
||||
|
||||
it { is_expected.to fail_a_contract }
|
||||
end
|
||||
|
||||
context "when arguments are valid" do
|
||||
context "when channel does not exist" do
|
||||
let(:channel_id) { -1 }
|
||||
|
||||
it { is_expected.to fail_to_find_a_model(:channel) }
|
||||
end
|
||||
|
||||
context "when channel is not a category channel" do
|
||||
fab!(:channel) { Fabricate(:direct_message_channel, auto_join_users: true) }
|
||||
|
||||
it { is_expected.to fail_to_find_a_model(:channel) }
|
||||
end
|
||||
|
||||
context "when channel is not in auto_join_users mode" do
|
||||
before { channel.update!(auto_join_users: false) }
|
||||
|
||||
it { is_expected.to fail_to_find_a_model(:channel) }
|
||||
end
|
||||
|
||||
context "when channel is found" do
|
||||
fab!(:users) { Fabricate.times(2, :user) }
|
||||
|
||||
let(:manager) { mock.responds_like_instance_of(Chat::ChannelMembershipManager) }
|
||||
|
||||
before do
|
||||
Chat::Action::CreateMembershipsForAutoJoin
|
||||
.stubs(:call)
|
||||
.with(has_entries(channel: channel, contract: instance_of(described_class::Contract)))
|
||||
.returns(user_ids)
|
||||
Chat::ChannelMembershipManager.stubs(:new).with(channel).returns(manager)
|
||||
manager.stubs(:recalculate_user_count)
|
||||
end
|
||||
|
||||
context "when more than one membership is created" do
|
||||
let(:user_ids) { users.map(&:id) }
|
||||
|
||||
it "does not recalculate user count" do
|
||||
manager.expects(:recalculate_user_count).never
|
||||
result
|
||||
end
|
||||
|
||||
it "publishes an event" do
|
||||
Chat::Publisher.expects(:publish_new_channel).with(channel, users)
|
||||
result
|
||||
end
|
||||
end
|
||||
|
||||
context "when only one membership is created" do
|
||||
let(:user_ids) { [users.first.id] }
|
||||
|
||||
it "recalculates user count" do
|
||||
manager.expects(:recalculate_user_count)
|
||||
result
|
||||
end
|
||||
|
||||
it "publishes an event" do
|
||||
Chat::Publisher.expects(:publish_new_channel).with(channel, [users.first])
|
||||
result
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue
Block a user