diff --git a/app/models/group.rb b/app/models/group.rb index d24d05f696b..5e63d19b97d 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -793,22 +793,26 @@ class Group < ActiveRecord::Base group_user = self.group_users.find_by(user: user) return false if group_user.blank? - has_webhooks = WebHook.active_web_hooks(:group_user) - payload = - WebHook.generate_payload(:group_user, group_user, WebHookGroupUserSerializer) if has_webhooks group_user.destroy trigger_user_removed_event(user) - if has_webhooks - WebHook.enqueue_hooks( - :group_user, - :user_removed_from_group, - id: group_user.id, - payload: payload, - ) - end + enqueue_user_removed_from_group_webhook_events(group_user) + true end + def enqueue_user_removed_from_group_webhook_events(group_user) + return if !WebHook.active_web_hooks(:group_user) + + payload = WebHook.generate_payload(:group_user, group_user, WebHookGroupUserSerializer) + + WebHook.enqueue_hooks( + :group_user, + :user_removed_from_group, + id: group_user.id, + payload: payload, + ) + end + def trigger_user_added_event(user, automatic) DiscourseEvent.trigger(:user_added_to_group, user, self, automatic: automatic) end @@ -866,14 +870,7 @@ class Group < ActiveRecord::Base User.where(id: user_ids).update_all(user_attributes) if user_attributes.present? # update group user count - DB.exec <<~SQL - UPDATE groups g - SET user_count = - (SELECT COUNT(gu.user_id) - FROM group_users gu - WHERE gu.group_id = g.id) - WHERE g.id = #{self.id}; - SQL + recalculate_user_count end if self.grant_trust_level.present? @@ -883,6 +880,31 @@ class Group < ActiveRecord::Base self end + def bulk_remove(user_ids) + Group.transaction do + group_users_to_be_destroyed = group_users.includes(:user).where(user_id: user_ids).destroy_all + group_users_to_be_destroyed.each do |group_user| + trigger_user_removed_event(group_user.user) + enqueue_user_removed_from_group_webhook_events(group_user) + end + end + + recalculate_user_count + + true + end + + def recalculate_user_count + DB.exec <<~SQL + UPDATE groups g + SET user_count = + (SELECT COUNT(gu.user_id) + FROM group_users gu + WHERE gu.group_id = g.id) + WHERE g.id = #{self.id}; + SQL + end + def add_automatically(user, subject: nil) if users.exclude?(user) && add(user) logger = GroupActionLogger.new(Discourse.system_user, self) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index da63a2e8ebd..bb876b7bb21 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -864,6 +864,20 @@ RSpec.describe Group do events = DiscourseEvent.track_events { group.remove(user) }.map { |e| e[:event_name] } expect(events).to include(:user_removed_from_group) end + + describe "with webhook" do + fab!(:group_user_web_hook) { Fabricate(:group_user_web_hook) } + + it "Enqueues webhook events" do + group.remove(user) + job_args = Jobs::EmitWebHookEvent.jobs.last["args"].first + + expect(job_args["event_name"]).to eq("user_removed_from_group") + payload = JSON.parse(job_args["payload"]) + expect(payload["group_id"]).to eq(group.id) + expect(payload["user_id"]).to eq(user.id) + end + end end describe "#add" do @@ -994,7 +1008,45 @@ RSpec.describe Group do expect { group.bulk_add([user.id, admin.id]) group.reload - }.to change { group.user_count }.by(2) + }.to change { group.user_count }.from(0).to(2) + end + end + + describe "#bulk_remove" do + it "removes multiple users from the group and doesn't error with user_ids not present" do + group.bulk_add([user.id, admin.id]) + + group.bulk_remove([user.id, admin.id, admin.id + 1]) + + expect(group.group_users.count).to be_zero + end + + it "updates group user count" do + group.bulk_add([user.id, admin.id]) + expect(group.reload.user_count).to eq(2) + + group.bulk_remove([user.id, admin.id]) + expect(group.reload.user_count).to eq(0) + end + + describe "with webhook" do + fab!(:group_user_web_hook) { Fabricate(:group_user_web_hook) } + + it "Enqueues user_removed_from_group webhook events for each group_user" do + group.bulk_add([user.id, admin.id]) + + group.bulk_remove([user.id, admin.id]) + Jobs::EmitWebHookEvent + .jobs + .last(2) + .each do |event| + job_args = event["args"].first + expect(job_args["event_name"]).to eq("user_removed_from_group") + payload = JSON.parse(job_args["payload"]) + expect(payload["group_id"]).to eq(group.id) + expect([user.id, admin.id]).to include(payload["user_id"]) + end + end end end