From 01392ab90c97db174d6561a47688a3f4a6f5179e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Thu, 17 Nov 2022 15:16:14 +0100 Subject: [PATCH] FIX: Delete associated channel upon category deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently when a category is deleted, if it has an associated chat channel, the latter won’t be deleted automatically. The fix is quite simple as we were simply missing a `dependent: :destroy` option on the existing relation. --- .../20221117142910_delete_orphaned_channels.rb | 9 +++++++++ plugins/chat/lib/extensions/category_extension.rb | 2 +- .../regular/auto_manage_channel_memberships_spec.rb | 10 +++++++--- plugins/chat/spec/models/category_spec.rb | 2 +- .../chat/spec/requests/categories_controller_spec.rb | 4 ++++ 5 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 plugins/chat/db/post_migrate/20221117142910_delete_orphaned_channels.rb diff --git a/plugins/chat/db/post_migrate/20221117142910_delete_orphaned_channels.rb b/plugins/chat/db/post_migrate/20221117142910_delete_orphaned_channels.rb new file mode 100644 index 00000000000..44f9d9751b5 --- /dev/null +++ b/plugins/chat/db/post_migrate/20221117142910_delete_orphaned_channels.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class DeleteOrphanedChannels < ActiveRecord::Migration[7.0] + def up + DB.exec( + "DELETE FROM chat_channels WHERE chatable_type = 'Category' AND type = 'CategoryChannel' AND NOT EXISTS (SELECT * FROM categories WHERE categories.id = chat_channels.chatable_id)", + ) + end +end diff --git a/plugins/chat/lib/extensions/category_extension.rb b/plugins/chat/lib/extensions/category_extension.rb index a424ac38106..d12ce387645 100644 --- a/plugins/chat/lib/extensions/category_extension.rb +++ b/plugins/chat/lib/extensions/category_extension.rb @@ -5,7 +5,7 @@ module Chat::CategoryExtension include Chatable - prepended { has_one :category_channel, as: :chatable } + prepended { has_one :category_channel, as: :chatable, dependent: :destroy } def cannot_delete_reason return I18n.t("category.cannot_delete.has_chat_channels") if category_channel diff --git a/plugins/chat/spec/jobs/regular/auto_manage_channel_memberships_spec.rb b/plugins/chat/spec/jobs/regular/auto_manage_channel_memberships_spec.rb index b89e7f2c1f4..1ea5470c7f9 100644 --- a/plugins/chat/spec/jobs/regular/auto_manage_channel_memberships_spec.rb +++ b/plugins/chat/spec/jobs/regular/auto_manage_channel_memberships_spec.rb @@ -103,9 +103,13 @@ describe Jobs::AutoManageChannelMemberships do end context "when chatable doesn’t exist anymore" do - before do - channel.chatable.destroy! - channel.reload + let(:channel) do + Fabricate( + :category_channel, + auto_join_users: true, + chatable_type: "Category", + chatable_id: -1, + ) end it "does nothing" do diff --git a/plugins/chat/spec/models/category_spec.rb b/plugins/chat/spec/models/category_spec.rb index 5a68f6449b2..bf16ff3e618 100644 --- a/plugins/chat/spec/models/category_spec.rb +++ b/plugins/chat/spec/models/category_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Category do let(:channel_class) { CategoryChannel } end - it { is_expected.to have_one(:category_channel) } + it { is_expected.to have_one(:category_channel).dependent(:destroy) } describe "#cannot_delete_reason" do subject(:reason) { category.cannot_delete_reason } diff --git a/plugins/chat/spec/requests/categories_controller_spec.rb b/plugins/chat/spec/requests/categories_controller_spec.rb index 798396d640c..431ded15610 100644 --- a/plugins/chat/spec/requests/categories_controller_spec.rb +++ b/plugins/chat/spec/requests/categories_controller_spec.rb @@ -24,6 +24,10 @@ RSpec.describe CategoriesController do it "deletes the category" do expect { destroy_category }.to change { Category.count }.by(-1) end + + it "deletes the associated channel" do + expect { destroy_category }.to change { CategoryChannel.count }.by(-1) + end end context "when channel has messages" do