From af6788fd331291d7205c2c27bf8b9281704124a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Fri, 11 Oct 2024 12:44:59 +0200 Subject: [PATCH] DEV: Extract leave logic to the `Chat::Channel` model Extracted from https://github.com/discourse/discourse/pull/29129. --- plugins/chat/app/models/chat/channel.rb | 1 + .../app/models/chat/direct_message_channel.rb | 11 +++++++ .../chat/app/services/chat/leave_channel.rb | 9 +----- .../spec/models/chat/category_channel_spec.rb | 10 ++++++ .../chat/direct_message_channel_spec.rb | 31 +++++++++++++++++++ 5 files changed, 54 insertions(+), 8 deletions(-) diff --git a/plugins/chat/app/models/chat/channel.rb b/plugins/chat/app/models/chat/channel.rb index d7d70697f69..dc7e9cd631d 100644 --- a/plugins/chat/app/models/chat/channel.rb +++ b/plugins/chat/app/models/chat/channel.rb @@ -141,6 +141,7 @@ module Chat def remove(user) Chat::ChannelMembershipManager.new(self).unfollow(user) end + alias leave remove def url "#{Discourse.base_url}/chat/c/#{self.slug || "-"}/#{self.id}" diff --git a/plugins/chat/app/models/chat/direct_message_channel.rb b/plugins/chat/app/models/chat/direct_message_channel.rb index 23eea91aa8b..b179c96f90c 100644 --- a/plugins/chat/app/models/chat/direct_message_channel.rb +++ b/plugins/chat/app/models/chat/direct_message_channel.rb @@ -3,6 +3,9 @@ module Chat class DirectMessageChannel < Channel alias_method :direct_message, :chatable + + delegate :group?, to: :direct_message, prefix: true, allow_nil: true + before_validation(on: :create) { self.threading_enabled = true } def direct_message_channel? @@ -24,5 +27,13 @@ module Chat def generate_auto_slug self.slug.blank? end + + def leave(user) + return super unless direct_message_group? + transaction do + membership_for(user)&.destroy! + direct_message.users.delete(user) + end + end end end diff --git a/plugins/chat/app/services/chat/leave_channel.rb b/plugins/chat/app/services/chat/leave_channel.rb index 63afd9ab3df..6316963980c 100644 --- a/plugins/chat/app/services/chat/leave_channel.rb +++ b/plugins/chat/app/services/chat/leave_channel.rb @@ -33,14 +33,7 @@ module Chat end def leave(channel:, guardian:) - ActiveRecord::Base.transaction do - if channel.direct_message_channel? && channel.chatable&.group - channel.membership_for(guardian.user)&.destroy! - channel.chatable.direct_message_users.where(user_id: guardian.user.id).destroy_all - else - channel.remove(guardian.user) - end - end + channel.leave(guardian.user) end def recompute_users_count(channel:) diff --git a/plugins/chat/spec/models/chat/category_channel_spec.rb b/plugins/chat/spec/models/chat/category_channel_spec.rb index b527bae77da..da464fa48bf 100644 --- a/plugins/chat/spec/models/chat/category_channel_spec.rb +++ b/plugins/chat/spec/models/chat/category_channel_spec.rb @@ -88,6 +88,16 @@ RSpec.describe Chat::CategoryChannel do end end + describe "#leave" do + let(:original_method) { channel.method(:remove) } + let(:aliased_method) { channel.method(:leave) } + + it "is an alias to '#remove'" do + expect(original_method.original_name).to eq(aliased_method.original_name) + expect(original_method.source_location).to eq(aliased_method.source_location) + end + end + describe "slug generation" do subject(:channel) { Fabricate(:category_channel) } diff --git a/plugins/chat/spec/models/chat/direct_message_channel_spec.rb b/plugins/chat/spec/models/chat/direct_message_channel_spec.rb index 2603678f648..f33c9d4e7a0 100644 --- a/plugins/chat/spec/models/chat/direct_message_channel_spec.rb +++ b/plugins/chat/spec/models/chat/direct_message_channel_spec.rb @@ -6,6 +6,7 @@ RSpec.describe Chat::DirectMessageChannel do it_behaves_like "a chat channel model" it { is_expected.to delegate_method(:allowed_user_ids).to(:direct_message).as(:user_ids) } + it { is_expected.to delegate_method(:group?).to(:direct_message).with_prefix.allow_nil } it { is_expected.to validate_length_of(:description).is_at_most(500) } it { is_expected.to validate_length_of(:chatable_type).is_at_most(100) } it { is_expected.to validate_length_of(:type).is_at_most(100) } @@ -70,6 +71,36 @@ RSpec.describe Chat::DirectMessageChannel do end end + describe "#leave" do + subject(:leave) { channel.leave(user) } + + let(:channel) { Fabricate(:direct_message_channel, group:) } + let(:user) { channel.chatable.users.first } + let(:membership) { channel.membership_for(user) } + + context "when DM is not a group" do + let(:group) { false } + + it "unfollows the channel for the provided user" do + expect { leave }.to change { membership.reload.following? }.to(false) + end + end + + context "when DM is a group" do + let(:group) { true } + + it "destroys the provided user’s membership" do + expect { leave }.to change { channel.user_chat_channel_memberships.where(user:).count }.by( + -1, + ) + end + + it "removes the provided user from the DM" do + expect { leave }.to change { channel.chatable.users.where(id: user).count }.by(-1) + end + end + end + describe "slug generation" do subject(:channel) { Fabricate(:direct_message_channel) }