From 9c1d1c4070a26864839798b5718155263f754f1a Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Fri, 8 May 2026 13:40:36 +0530 Subject: [PATCH] feat(labels): remove label associations asynchronously on delete (#13531) ## Summary - Remove label deletion dependency on association cleanup by deleting immediately and enqueueing a background job. - Add `Labels::RemoveAssociationsJob` to strip deleted label references from tagged conversations and contacts. - Keep this version simple by removing the label count/prompt requirement requested. ## Implementation notes - Enqueue job from `Api::V1::Accounts::LabelsController#destroy` with label title + account id. - Background work performed in `Labels::DestroyService`. ## References - Linear issue: https://linear.app/chatwoot/issue/CW-4765/cw-2857-enhancement-removing-labels-is-inconsistent - GitHub issue: https://github.com/chatwoot/chatwoot/issues/1249 ## Testing - `bundle exec rspec spec/controllers/api/v1/accounts/labels_controller_spec.rb spec/services/labels/destroy_service_spec.rb spec/jobs/labels/remove_associations_job_spec.rb spec/services/labels/update_service_spec.rb` - `bundle exec rubocop app/controllers/api/v1/accounts/labels_controller.rb app/jobs/labels/remove_associations_job.rb spec/controllers/api/v1/accounts/labels_controller_spec.rb spec/jobs/labels/remove_associations_job_spec.rb spec/services/labels/destroy_service_spec.rb` --------- Co-authored-by: Sony Mathew Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com> --- .../api/v1/accounts/labels_controller.rb | 9 ++ app/jobs/labels/remove_associations_job.rb | 11 ++ app/services/labels/destroy_service.rb | 60 +++++++++++ .../api/v1/accounts/labels_controller_spec.rb | 36 +++++++ .../labels/remove_associations_job_spec.rb | 21 ++++ spec/services/labels/destroy_service_spec.rb | 102 ++++++++++++++++++ 6 files changed, 239 insertions(+) create mode 100644 app/jobs/labels/remove_associations_job.rb create mode 100644 app/services/labels/destroy_service.rb create mode 100644 spec/jobs/labels/remove_associations_job_spec.rb create mode 100644 spec/services/labels/destroy_service_spec.rb diff --git a/app/controllers/api/v1/accounts/labels_controller.rb b/app/controllers/api/v1/accounts/labels_controller.rb index 54455943b..6889d30a4 100644 --- a/app/controllers/api/v1/accounts/labels_controller.rb +++ b/app/controllers/api/v1/accounts/labels_controller.rb @@ -18,7 +18,16 @@ class Api::V1::Accounts::LabelsController < Api::V1::Accounts::BaseController end def destroy + label_title = @label.title + account_id = Current.account.id + label_deleted_at = Time.current + @label.destroy! + Labels::RemoveAssociationsJob.perform_later( + label_title: label_title, + account_id: account_id, + label_deleted_at: label_deleted_at + ) head :ok end diff --git a/app/jobs/labels/remove_associations_job.rb b/app/jobs/labels/remove_associations_job.rb new file mode 100644 index 000000000..502d9339e --- /dev/null +++ b/app/jobs/labels/remove_associations_job.rb @@ -0,0 +1,11 @@ +class Labels::RemoveAssociationsJob < ApplicationJob + queue_as :default + + def perform(label_title:, account_id:, label_deleted_at:) + Labels::DestroyService.new( + label_title: label_title, + account_id: account_id, + label_deleted_at: label_deleted_at + ).perform + end +end diff --git a/app/services/labels/destroy_service.rb b/app/services/labels/destroy_service.rb new file mode 100644 index 000000000..080e708e7 --- /dev/null +++ b/app/services/labels/destroy_service.rb @@ -0,0 +1,60 @@ +class Labels::DestroyService + pattr_initialize [:label_title!, :account_id!, :label_deleted_at!] + + def perform + remove_conversation_labels + remove_contact_labels + end + + private + + def remove_conversation_labels + tagged_conversations.find_in_batches do |conversation_batch| + conversation_batch.each do |conversation| + update_conversation_cached_labels(conversation) + end + delete_label_taggings('Conversation', conversation_batch.map(&:id)) + end + end + + def remove_contact_labels + contact_label_taggings.in_batches do |tagging_batch| + ActsAsTaggableOn::Tagging.where(id: tagging_batch.select(:id)).delete_all + end + end + + def update_conversation_cached_labels(conversation) + label_list = conversation.label_list.dup + label_list.remove(label_title) + + # We only want the acts-as-taggable-on cache effect here, not Conversation callbacks/events. + # rubocop:disable Rails/SkipsModelValidations + conversation.update_column(:cached_label_list, label_list.join("#{ActsAsTaggableOn.delimiter} ")) + # rubocop:enable Rails/SkipsModelValidations + end + + def tagged_conversations + account.conversations.where(id: label_taggings_for('Conversation').select(:taggable_id)) + end + + def contact_label_taggings + label_taggings_for('Contact').where(taggable_id: account.contacts.select(:id)) + end + + def delete_label_taggings(taggable_type, taggable_ids) + ActsAsTaggableOn::Tagging + .where(id: label_taggings_for(taggable_type).where(taggable_id: taggable_ids).select(:id)) + .delete_all + end + + def label_taggings_for(taggable_type) + ActsAsTaggableOn::Tagging + .joins(:tag) + .where(context: 'labels', taggable_type: taggable_type, tags: { name: label_title }) + .where('taggings.created_at <= ?', label_deleted_at) + end + + def account + @account ||= Account.find(account_id) + end +end diff --git a/spec/controllers/api/v1/accounts/labels_controller_spec.rb b/spec/controllers/api/v1/accounts/labels_controller_spec.rb index 61751ad68..657b53796 100644 --- a/spec/controllers/api/v1/accounts/labels_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/labels_controller_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' RSpec.describe 'Label API', type: :request do let!(:account) { create(:account) } let!(:label) { create(:label, account: account) } + let!(:conversation) { create(:conversation, account: account) } describe 'GET /api/v1/accounts/{account.id}/labels' do context 'when it is an unauthenticated user' do @@ -101,4 +102,39 @@ RSpec.describe 'Label API', type: :request do end end end + + describe 'DELETE /api/v1/accounts/{account.id}/labels/:id' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + delete "/api/v1/accounts/#{account.id}/labels/#{label.id}" + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + let(:admin) { create(:user, account: account, role: :administrator) } + + it 'deletes the label and enqueues label cleanup' do + label_deleted_at = Time.zone.parse('2026-05-07 10:00:00 UTC') + conversation.label_list.add(label.title) + conversation.save! + + clear_enqueued_jobs + + travel_to(label_deleted_at) do + expect do + delete "/api/v1/accounts/#{account.id}/labels/#{label.id}", headers: admin.create_new_auth_token, as: :json + end.to have_enqueued_job(Labels::RemoveAssociationsJob).with( + label_title: label.title, + account_id: account.id, + label_deleted_at: label_deleted_at + ) + end + + expect(response).to have_http_status(:ok) + expect(Label.exists?(label.id)).to be(false) + end + end + end end diff --git a/spec/jobs/labels/remove_associations_job_spec.rb b/spec/jobs/labels/remove_associations_job_spec.rb new file mode 100644 index 000000000..0b608630f --- /dev/null +++ b/spec/jobs/labels/remove_associations_job_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Labels::RemoveAssociationsJob do + subject(:job) do + described_class.perform_later( + label_title: label_title, + account_id: account_id, + label_deleted_at: label_deleted_at + ) + end + + let(:label_title) { 'billing' } + let(:account_id) { 1 } + let(:label_deleted_at) { Time.current } + + it 'queues the job' do + expect { job }.to have_enqueued_job(described_class) + .with(label_title: label_title, account_id: account_id, label_deleted_at: label_deleted_at) + .on_queue('default') + end +end diff --git a/spec/services/labels/destroy_service_spec.rb b/spec/services/labels/destroy_service_spec.rb new file mode 100644 index 000000000..7d06b72d0 --- /dev/null +++ b/spec/services/labels/destroy_service_spec.rb @@ -0,0 +1,102 @@ +require 'rails_helper' + +describe Labels::DestroyService do + let(:account) { create(:account) } + let(:conversation) { create(:conversation, account: account) } + let(:label) { create(:label, account: account) } + let(:contact) { conversation.contact } + let(:label_deleted_at) { Time.zone.parse('2026-05-07 10:00:00 UTC') } + + before do + conversation.label_list.add(label.title) + conversation.label_list.add('billing') + conversation.save! + + contact.label_list.add(label.title) + contact.label_list.add('vip') + contact.save! + + set_label_tagging_created_at(conversation, label_deleted_at - 1.minute) + set_label_tagging_created_at(contact, label_deleted_at - 1.minute) + end + + describe '#perform' do + it 'removes label from associated conversations and contacts' do + described_class.new( + label_title: label.title, + account_id: account.id, + label_deleted_at: label_deleted_at + ).perform + + expect(conversation.reload.label_list).to eq(['billing']) + expect(conversation.cached_label_list).to eq('billing') + expect(contact.reload.label_list).to eq(['vip']) + end + + it 'removes label associations after the label record is destroyed' do + label_title = label.title + label.destroy! + + described_class.new( + label_title: label_title, + account_id: account.id, + label_deleted_at: label_deleted_at + ).perform + + expect(conversation.reload.label_list).to eq(['billing']) + expect(conversation.cached_label_list).to eq('billing') + expect(contact.reload.label_list).to eq(['vip']) + end + + it 'does not remove labels from other accounts' do + other_account = create(:account) + other_conversation = create(:conversation, account: other_account) + other_conversation.label_list.add(label.title) + other_conversation.save! + set_label_tagging_created_at(other_conversation, label_deleted_at - 1.minute) + + described_class.new( + label_title: label.title, + account_id: account.id, + label_deleted_at: label_deleted_at + ).perform + + expect(other_conversation.reload.label_list).to eq([label.title]) + end + + it 'does not dispatch conversation or contact update events' do + expect(Rails.configuration.dispatcher).not_to receive(:dispatch) + + described_class.new( + label_title: label.title, + account_id: account.id, + label_deleted_at: label_deleted_at + ).perform + end + + it 'does not remove label associations created after the label was deleted' do + other_conversation = create(:conversation, account: account) + other_conversation.label_list.add(label.title) + other_conversation.save! + set_label_tagging_created_at(other_conversation, label_deleted_at + 1.minute) + + described_class.new( + label_title: label.title, + account_id: account.id, + label_deleted_at: label_deleted_at + ).perform + + expect(conversation.reload.label_list).to eq(['billing']) + expect(conversation.cached_label_list).to eq('billing') + expect(contact.reload.label_list).to eq(['vip']) + expect(other_conversation.reload.label_list).to eq([label.title]) + end + end + + def set_label_tagging_created_at(record, created_at) + ActsAsTaggableOn::Tagging + .joins(:tag) + .find_by!(context: 'labels', taggable: record, tags: { name: label.title }) + .update!(created_at: created_at) + end +end