From 7a67799f35430397e5c098588007a52986fb8b28 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 11 Jun 2025 22:45:32 +0530 Subject: [PATCH] fix: Send CSAT survey only when agent can reply in conversation (#11637) --- app/listeners/csat_survey_listener.rb | 8 ++ app/services/csat_survey_service.rb | 48 ++++++++++ .../hook_execution_service.rb | 19 ---- config/locales/en.yml | 2 + spec/listeners/csat_survey_listener_spec.rb | 31 +++++++ spec/services/csat_survey_service_spec.rb | 91 +++++++++++++++++++ .../hook_execution_service_spec.rb | 63 ------------- 7 files changed, 180 insertions(+), 82 deletions(-) create mode 100644 app/services/csat_survey_service.rb create mode 100644 spec/services/csat_survey_service_spec.rb diff --git a/app/listeners/csat_survey_listener.rb b/app/listeners/csat_survey_listener.rb index abc366400..82cf09497 100644 --- a/app/listeners/csat_survey_listener.rb +++ b/app/listeners/csat_survey_listener.rb @@ -1,4 +1,12 @@ class CsatSurveyListener < BaseListener + def conversation_status_changed(event) + conversation = extract_conversation_and_account(event)[0] + + return unless conversation.resolved? + + CsatSurveyService.new(conversation: conversation).perform + end + def message_updated(event) message = extract_message_and_account(event)[0] return unless message.input_csat? diff --git a/app/services/csat_survey_service.rb b/app/services/csat_survey_service.rb new file mode 100644 index 000000000..15d63722c --- /dev/null +++ b/app/services/csat_survey_service.rb @@ -0,0 +1,48 @@ +class CsatSurveyService + pattr_initialize [:conversation!] + + def perform + return unless should_send_csat_survey? + + if within_messaging_window? + ::MessageTemplates::Template::CsatSurvey.new(conversation: conversation).perform + else + create_csat_not_sent_activity_message + end + end + + private + + delegate :inbox, :contact, to: :conversation + + def should_send_csat_survey? + conversation_allows_csat? && csat_enabled? && !csat_already_sent? + end + + def conversation_allows_csat? + conversation.resolved? && !conversation.tweet? + end + + def csat_enabled? + inbox.csat_survey_enabled? + end + + def csat_already_sent? + conversation.messages.where(content_type: :input_csat).present? + end + + def within_messaging_window? + conversation.can_reply? + end + + def create_csat_not_sent_activity_message + content = I18n.t('conversations.activity.csat.not_sent_due_to_messaging_window') + activity_message_params = { + account_id: conversation.account_id, + inbox_id: conversation.inbox_id, + message_type: :activity, + content: content + } + ::Conversations::ActivityMessageJob.perform_later(conversation, activity_message_params) if content + end +end diff --git a/app/services/message_templates/hook_execution_service.rb b/app/services/message_templates/hook_execution_service.rb index a8a4c4318..7da475af3 100644 --- a/app/services/message_templates/hook_execution_service.rb +++ b/app/services/message_templates/hook_execution_service.rb @@ -17,7 +17,6 @@ class MessageTemplates::HookExecutionService ::MessageTemplates::Template::OutOfOffice.new(conversation: conversation).perform if should_send_out_of_office_message? ::MessageTemplates::Template::Greeting.new(conversation: conversation).perform if should_send_greeting? ::MessageTemplates::Template::EmailCollect.new(conversation: conversation).perform if inbox.enable_email_collect && should_send_email_collect? - ::MessageTemplates::Template::CsatSurvey.new(conversation: conversation).perform if should_send_csat_survey? end def should_send_out_of_office_message? @@ -55,23 +54,5 @@ class MessageTemplates::HookExecutionService def contact_has_email? contact.email end - - def csat_enabled_conversation? - return false unless conversation.resolved? - # should not sent since the link will be public - return false if conversation.tweet? - return false unless inbox.csat_survey_enabled? - - true - end - - def should_send_csat_survey? - return unless csat_enabled_conversation? - - # only send CSAT once in a conversation - return if conversation.messages.where(content_type: :input_csat).present? - - true - end end MessageTemplates::HookExecutionService.prepend_mod_with('MessageTemplates::HookExecutionService') diff --git a/config/locales/en.yml b/config/locales/en.yml index 74177f7dd..77f2af9b4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -186,6 +186,8 @@ en: sla: added: '%{user_name} added SLA policy %{sla_name}' removed: '%{user_name} removed SLA policy %{sla_name}' + csat: + not_sent_due_to_messaging_window: 'CSAT survey not sent due to outgoing message restrictions' muted: '%{user_name} has muted the conversation' unmuted: '%{user_name} has unmuted the conversation' auto_resolution_message: 'Resolving the conversation as it has been inactive for a while. Please start a new conversation if you need further assistance.' diff --git a/spec/listeners/csat_survey_listener_spec.rb b/spec/listeners/csat_survey_listener_spec.rb index 9161c4e71..0f957dcb4 100644 --- a/spec/listeners/csat_survey_listener_spec.rb +++ b/spec/listeners/csat_survey_listener_spec.rb @@ -13,6 +13,8 @@ describe CsatSurveyListener do ) end let!(:event) { Events::Base.new(event_name, Time.zone.now, message: message) } + let!(:csat_enabled_inbox) { create(:inbox, account: account, csat_survey_enabled: true) } + let!(:resolved_conversation) { create(:conversation, account: account, inbox: csat_enabled_inbox, status: :resolved) } describe '#message_updated' do let(:event_name) { 'message.updated' } @@ -33,4 +35,33 @@ describe CsatSurveyListener do end end end + + describe '#conversation_status_changed' do + let(:event_name) { 'conversation.status_changed' } + let(:csat_service) { double } + + before do + allow(resolved_conversation).to receive(:saved_change_to_status?).and_return(true) + end + + context 'when conversation is resolved and CSAT is enabled' do + it 'triggers CSAT survey service' do + event = Events::Base.new(event_name, Time.zone.now, conversation: resolved_conversation) + expect(csat_service).to receive(:perform) + expect(CsatSurveyService).to receive(:new).with(conversation: resolved_conversation).and_return(csat_service) + + listener.conversation_status_changed(event) + end + end + + context 'when conversation is not resolved' do + it 'does not trigger CSAT survey service' do + open_conversation = create(:conversation, account: account, inbox: csat_enabled_inbox, status: :open) + event = Events::Base.new(event_name, Time.zone.now, conversation: open_conversation) + + expect(CsatSurveyService).not_to receive(:new) + listener.conversation_status_changed(event) + end + end + end end diff --git a/spec/services/csat_survey_service_spec.rb b/spec/services/csat_survey_service_spec.rb new file mode 100644 index 000000000..5b3b3279a --- /dev/null +++ b/spec/services/csat_survey_service_spec.rb @@ -0,0 +1,91 @@ +require 'rails_helper' + +describe CsatSurveyService do + let(:account) { create(:account) } + let(:inbox) { create(:inbox, account: account, csat_survey_enabled: true) } + let(:conversation) { create(:conversation, inbox: inbox, account: account, status: :resolved) } + let(:service) { described_class.new(conversation: conversation) } + + describe '#perform' do + let(:csat_template) { instance_double(MessageTemplates::Template::CsatSurvey) } + + before do + allow(MessageTemplates::Template::CsatSurvey).to receive(:new).and_return(csat_template) + allow(csat_template).to receive(:perform) + allow(Conversations::ActivityMessageJob).to receive(:perform_later) + end + + context 'when CSAT survey should be sent' do + before do + allow(conversation).to receive(:can_reply?).and_return(true) + end + + it 'sends CSAT survey when within messaging window' do + service.perform + + expect(MessageTemplates::Template::CsatSurvey).to have_received(:new).with(conversation: conversation) + expect(csat_template).to have_received(:perform) + end + end + + context 'when outside messaging window' do + before do + allow(conversation).to receive(:can_reply?).and_return(false) + end + + it 'creates activity message instead of sending survey' do + service.perform + + expect(Conversations::ActivityMessageJob).to have_received(:perform_later).with( + conversation, + hash_including(content: I18n.t('conversations.activity.csat.not_sent_due_to_messaging_window')) + ) + expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new) + end + end + + context 'when CSAT survey should not be sent' do + it 'does nothing when conversation is not resolved' do + conversation.update(status: :open) + + service.perform + + expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new) + expect(Conversations::ActivityMessageJob).not_to have_received(:perform_later) + end + + it 'does nothing when CSAT survey is not enabled' do + inbox.update(csat_survey_enabled: false) + + service.perform + + expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new) + expect(Conversations::ActivityMessageJob).not_to have_received(:perform_later) + end + + it 'does nothing when CSAT already sent' do + create(:message, conversation: conversation, content_type: :input_csat) + + service.perform + + expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new) + expect(Conversations::ActivityMessageJob).not_to have_received(:perform_later) + end + + it 'does nothing for Twitter conversations' do + twitter_channel = create(:channel_twitter_profile) + twitter_inbox = create(:inbox, channel: twitter_channel, csat_survey_enabled: true) + twitter_conversation = create(:conversation, + inbox: twitter_inbox, + status: :resolved, + additional_attributes: { type: 'tweet' }) + twitter_service = described_class.new(conversation: twitter_conversation) + + twitter_service.perform + + expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new) + expect(Conversations::ActivityMessageJob).not_to have_received(:perform_later) + end + end + end +end \ No newline at end of file diff --git a/spec/services/message_templates/hook_execution_service_spec.rb b/spec/services/message_templates/hook_execution_service_spec.rb index 24e40ea8d..b80c42fd8 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -111,69 +111,6 @@ describe MessageTemplates::HookExecutionService do end end - context 'when CSAT Survey' do - let(:csat_survey) { double } - let(:conversation) { create(:conversation) } - - before do - allow(MessageTemplates::Template::CsatSurvey).to receive(:new).and_return(csat_survey) - allow(csat_survey).to receive(:perform).and_return(true) - create(:message, conversation: conversation, message_type: 'incoming') - end - - it 'calls ::MessageTemplates::Template::CsatSurvey when a conversation is resolved in an inbox with survey enabled' do - conversation.inbox.update(csat_survey_enabled: true) - - conversation.resolved! - Conversations::ActivityMessageJob.perform_now(conversation, - { account_id: conversation.account_id, inbox_id: conversation.inbox_id, message_type: :activity, - content: 'Conversation marked resolved!!' }) - - expect(MessageTemplates::Template::CsatSurvey).to have_received(:new).with(conversation: conversation) - expect(csat_survey).to have_received(:perform) - end - - it 'will not call ::MessageTemplates::Template::CsatSurvey when Csat is not enabled' do - conversation.inbox.update(csat_survey_enabled: false) - - conversation.resolved! - Conversations::ActivityMessageJob.perform_now(conversation, - { account_id: conversation.account_id, inbox_id: conversation.inbox_id, message_type: :activity, - content: 'Conversation marked resolved!!' }) - - expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new).with(conversation: conversation) - expect(csat_survey).not_to have_received(:perform) - end - - it 'will not call ::MessageTemplates::Template::CsatSurvey if its a tweet conversation' do - twitter_channel = create(:channel_twitter_profile) - twitter_inbox = create(:inbox, channel: twitter_channel) - conversation = create(:conversation, inbox: twitter_inbox, additional_attributes: { type: 'tweet' }) - conversation.inbox.update(csat_survey_enabled: true) - - conversation.resolved! - Conversations::ActivityMessageJob.perform_now(conversation, - { account_id: conversation.account_id, inbox_id: conversation.inbox_id, message_type: :activity, - content: 'Conversation marked resolved!!' }) - - expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new).with(conversation: conversation) - expect(csat_survey).not_to have_received(:perform) - end - - it 'will not call ::MessageTemplates::Template::CsatSurvey if another Csat was already sent' do - conversation.inbox.update(csat_survey_enabled: true) - conversation.messages.create!(message_type: 'outgoing', content_type: :input_csat, account: conversation.account, inbox: conversation.inbox) - - conversation.resolved! - Conversations::ActivityMessageJob.perform_now(conversation, - { account_id: conversation.account_id, inbox_id: conversation.inbox_id, message_type: :activity, - content: 'Conversation marked resolved!!' }) - - expect(MessageTemplates::Template::CsatSurvey).not_to have_received(:new).with(conversation: conversation) - expect(csat_survey).not_to have_received(:perform) - end - end - context 'when it is after working hours' do it 'calls ::MessageTemplates::Template::OutOfOffice' do contact = create(:contact)