From d526cf283d610274fe321ee87076f3b5b936f67f Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Mon, 12 Jan 2026 13:15:40 +0530 Subject: [PATCH 1/2] fix: pass serialized data in notification.deleted event to avoid Deserialisation (#13061) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://one.newrelic.com/alerts/issue?account=3437125&duration=259200000&state=d088e9b7-d0ce-3fcf-fda5-145df8b9cb2a ## Description Pass serialized data instead of ActiveRecord object in dispatch_destroy_event to prevent ActiveJob::DeserializationError when the notification is already deleted. This error occurs frequently because RemoveDuplicateNotificationJob deletes notifications, and by the time the async EventDispatcherJob runs, the record no longer exists. ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- > [!NOTE] > Avoids ActiveJob deserialization failures by sending serialized data for notification deletion and updating the listener accordingly. > > - `Notification#dispatch_destroy_event` now dispatches `NOTIFICATION_DELETED` with serialized `notification_data` (`id`, `user_id`, `account_id`) instead of the AR object > - `ActionCableListener#notification_deleted` reads `notification_data`, finds `User`/`Account`, computes `unread_count`/`count` via `NotificationFinder`, and broadcasts using the user’s pubsub token > - Specs updated to pass `notification_data` and assert payload (including `unread_count`/`count`) > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e2ffbe765b148fdfd2cd2e031c657c36e423c1f5. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --------- Co-authored-by: Vishnu Narayanan --- app/listeners/action_cable_listener.rb | 16 ++++++++++++---- app/models/notification.rb | 12 +++++++++++- spec/listeners/action_cable_listener_spec.rb | 9 ++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/app/listeners/action_cable_listener.rb b/app/listeners/action_cable_listener.rb index 109f6d344..48b7a3aa9 100644 --- a/app/listeners/action_cable_listener.rb +++ b/app/listeners/action_cable_listener.rb @@ -14,11 +14,19 @@ class ActionCableListener < BaseListener end def notification_deleted(event) - return if event.data[:notification].user.blank? + notification_data = event.data[:notification_data] - notification, account, unread_count, count = extract_notification_and_account(event) - tokens = [event.data[:notification].user.pubsub_token] - broadcast(account, tokens, NOTIFICATION_DELETED, { notification: { id: notification.id }, unread_count: unread_count, count: count }) + user = User.find_by(id: notification_data[:user_id]) + account = Account.find_by(id: notification_data[:account_id]) + return if user.blank? || account.blank? + + notification_finder = NotificationFinder.new(user, account) + tokens = [user.pubsub_token] + broadcast(account, tokens, NOTIFICATION_DELETED, { + notification: { id: notification_data[:id] }, + unread_count: notification_finder.unread_count, + count: notification_finder.count + }) end def account_cache_invalidated(event) diff --git a/app/models/notification.rb b/app/models/notification.rb index db07e3679..806eabdf1 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -180,7 +180,17 @@ class Notification < ApplicationRecord end def dispatch_destroy_event - Rails.configuration.dispatcher.dispatch(NOTIFICATION_DELETED, Time.zone.now, notification: self) + # Pass serialized data instead of ActiveRecord object to avoid DeserializationError + # when the async EventDispatcherJob runs after the notification has been deleted + Rails.configuration.dispatcher.dispatch( + NOTIFICATION_DELETED, + Time.zone.now, + notification_data: { + id: id, + user_id: user_id, + account_id: account_id + } + ) end def set_last_activity_at diff --git a/spec/listeners/action_cable_listener_spec.rb b/spec/listeners/action_cable_listener_spec.rb index 55b74116c..61f818da1 100644 --- a/spec/listeners/action_cable_listener_spec.rb +++ b/spec/listeners/action_cable_listener_spec.rb @@ -132,7 +132,14 @@ describe ActionCableListener do describe '#notification_deleted' do let(:event_name) { :'notification.deleted' } let!(:notification) { create(:notification, account: account, user: agent) } - let!(:event) { Events::Base.new(event_name, Time.zone.now, notification: notification) } + let(:notification_data) do + { + id: notification.id, + user_id: agent.id, + account_id: account.id + } + end + let!(:event) { Events::Base.new(event_name, Time.zone.now, notification_data: notification_data) } it 'sends message to account admins, inbox agents' do expect(ActionCableBroadcastJob).to receive(:perform_later).with( From b099d3a1eb7d08667b527da56e8bbef8786e2a48 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 12 Jan 2026 15:22:15 +0530 Subject: [PATCH 2/2] fix: PDF errors not loading in CI (#13236) --- .../llm/paginated_faq_generator_service.rb | 2 +- .../captain/llm/pdf_processing_service.rb | 2 +- lib/custom_exceptions/pdf.rb | 19 ++++++++++++++ lib/custom_exceptions/pdf_processing_error.rb | 25 ------------------- .../paginated_faq_generator_service_spec.rb | 3 +-- .../llm/pdf_processing_service_spec.rb | 3 +-- 6 files changed, 23 insertions(+), 31 deletions(-) create mode 100644 lib/custom_exceptions/pdf.rb delete mode 100644 lib/custom_exceptions/pdf_processing_error.rb diff --git a/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb b/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb index 149152107..93957a7f3 100644 --- a/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb +++ b/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb @@ -19,7 +19,7 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::LegacyBaseOpenAiService end def generate - raise CustomExceptions::PdfFaqGenerationError, I18n.t('captain.documents.missing_openai_file_id') if @document&.openai_file_id.blank? + raise CustomExceptions::Pdf::FaqGenerationError, I18n.t('captain.documents.missing_openai_file_id') if @document&.openai_file_id.blank? generate_paginated_faqs end diff --git a/enterprise/app/services/captain/llm/pdf_processing_service.rb b/enterprise/app/services/captain/llm/pdf_processing_service.rb index 82e3e9fbc..ac35a2848 100644 --- a/enterprise/app/services/captain/llm/pdf_processing_service.rb +++ b/enterprise/app/services/captain/llm/pdf_processing_service.rb @@ -10,7 +10,7 @@ class Captain::Llm::PdfProcessingService < Llm::LegacyBaseOpenAiService return if document.openai_file_id.present? file_id = upload_pdf_to_openai - raise CustomExceptions::PdfUploadError, I18n.t('captain.documents.pdf_upload_failed') if file_id.blank? + raise CustomExceptions::Pdf::UploadError, I18n.t('captain.documents.pdf_upload_failed') if file_id.blank? document.store_openai_file_id(file_id) end diff --git a/lib/custom_exceptions/pdf.rb b/lib/custom_exceptions/pdf.rb new file mode 100644 index 000000000..e6dfa7c41 --- /dev/null +++ b/lib/custom_exceptions/pdf.rb @@ -0,0 +1,19 @@ +module CustomExceptions::Pdf + class UploadError < CustomExceptions::Base + def initialize(message = 'PDF upload failed') + super(message) + end + end + + class ValidationError < CustomExceptions::Base + def initialize(message = 'PDF validation failed') + super(message) + end + end + + class FaqGenerationError < CustomExceptions::Base + def initialize(message = 'PDF FAQ generation failed') + super(message) + end + end +end diff --git a/lib/custom_exceptions/pdf_processing_error.rb b/lib/custom_exceptions/pdf_processing_error.rb deleted file mode 100644 index 2c81f95d8..000000000 --- a/lib/custom_exceptions/pdf_processing_error.rb +++ /dev/null @@ -1,25 +0,0 @@ -module CustomExceptions - class PdfProcessingError < Base - def initialize(message = 'PDF processing failed') - super(message) - end - end - - class PdfUploadError < PdfProcessingError - def initialize(message = 'PDF upload failed') - super(message) - end - end - - class PdfValidationError < PdfProcessingError - def initialize(message = 'PDF validation failed') - super(message) - end - end - - class PdfFaqGenerationError < PdfProcessingError - def initialize(message = 'PDF FAQ generation failed') - super(message) - end - end -end diff --git a/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb b/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb index 5215a2d40..ca4518435 100644 --- a/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb +++ b/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb @@ -1,5 +1,4 @@ require 'rails_helper' -require 'custom_exceptions/pdf_processing_error' RSpec.describe Captain::Llm::PaginatedFaqGeneratorService do let(:document) { create(:captain_document) } @@ -23,7 +22,7 @@ RSpec.describe Captain::Llm::PaginatedFaqGeneratorService do end it 'raises an error' do - expect { service.generate }.to raise_error(CustomExceptions::PdfFaqGenerationError) + expect { service.generate }.to raise_error(CustomExceptions::Pdf::FaqGenerationError) end end diff --git a/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb b/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb index fd519dcdd..7e9ec9a40 100644 --- a/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb +++ b/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb @@ -1,5 +1,4 @@ require 'rails_helper' -require 'custom_exceptions/pdf_processing_error' RSpec.describe Captain::Llm::PdfProcessingService do let(:document) { create(:captain_document) } @@ -52,7 +51,7 @@ RSpec.describe Captain::Llm::PdfProcessingService do it 'raises error when upload fails' do allow(mock_client.files).to receive(:upload).and_return({ 'id' => nil }) - expect { service.process }.to raise_error(CustomExceptions::PdfUploadError) + expect { service.process }.to raise_error(CustomExceptions::Pdf::UploadError) end end end