From 70c29f699c4bb8a1b0ff560fdb8577c5414bacc6 Mon Sep 17 00:00:00 2001 From: Pranav Date: Thu, 29 May 2025 16:52:33 -0600 Subject: [PATCH] =?UTF-8?q?fix(revert):=20"fix:=20Send=20CSAT=20survey=20o?= =?UTF-8?q?nly=20when=20agent=20can=20reply=20in=20conversati=E2=80=A6=20(?= =?UTF-8?q?#11634)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …on (#11584)" This reverts commit b5ebc4763723e15ad3a04ab0b0ecb1e01087a2b1. # Pull Request Template ## Description Please include a summary of the change and issue(s) fixed. Also, mention relevant motivation, context, and any dependencies that this change requires. Fixes # (issue) ## Type of change Please delete options that are not relevant. - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. ## 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 --- .../concerns/activity_message_handler.rb | 1 - .../concerns/csat_activity_message_handler.rb | 8 ----- .../hook_execution_service.rb | 23 ++++----------- config/locales/en.yml | 2 -- spec/models/conversation_spec.rb | 14 --------- .../hook_execution_service_spec.rb | 29 +------------------ 6 files changed, 6 insertions(+), 71 deletions(-) delete mode 100644 app/models/concerns/csat_activity_message_handler.rb diff --git a/app/models/concerns/activity_message_handler.rb b/app/models/concerns/activity_message_handler.rb index 0d6741c7a..54e58b4d9 100644 --- a/app/models/concerns/activity_message_handler.rb +++ b/app/models/concerns/activity_message_handler.rb @@ -5,7 +5,6 @@ module ActivityMessageHandler include LabelActivityMessageHandler include SlaActivityMessageHandler include TeamActivityMessageHandler - include CsatActivityMessageHandler private diff --git a/app/models/concerns/csat_activity_message_handler.rb b/app/models/concerns/csat_activity_message_handler.rb deleted file mode 100644 index 7a2488c4d..000000000 --- a/app/models/concerns/csat_activity_message_handler.rb +++ /dev/null @@ -1,8 +0,0 @@ -module CsatActivityMessageHandler - extend ActiveSupport::Concern - - def create_csat_not_sent_activity_message - content = I18n.t('conversations.activity.csat.not_sent_due_to_messaging_window') - ::Conversations::ActivityMessageJob.perform_later(self, activity_message_params(content)) 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 8291c8a9c..a8a4c4318 100644 --- a/app/services/message_templates/hook_execution_service.rb +++ b/app/services/message_templates/hook_execution_service.rb @@ -17,7 +17,7 @@ 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? - handle_csat_survey + ::MessageTemplates::Template::CsatSurvey.new(conversation: conversation).perform if should_send_csat_survey? end def should_send_out_of_office_message? @@ -65,26 +65,13 @@ class MessageTemplates::HookExecutionService true end - def handle_csat_survey + def should_send_csat_survey? return unless csat_enabled_conversation? + # only send CSAT once in a conversation - return if csat_already_sent? + return if conversation.messages.where(content_type: :input_csat).present? - # Only send CSAT if agent can still reply by checking the messaging window restriction - # https://www.chatwoot.com/docs/self-hosted/supported-features#outgoing-message-restriction - if within_messaging_window? - ::MessageTemplates::Template::CsatSurvey.new(conversation: conversation).perform - else - conversation.create_csat_not_sent_activity_message - end - end - - def csat_already_sent? - conversation.messages.where(content_type: :input_csat).present? - end - - def within_messaging_window? - conversation.can_reply? + true end end MessageTemplates::HookExecutionService.prepend_mod_with('MessageTemplates::HookExecutionService') diff --git a/config/locales/en.yml b/config/locales/en.yml index b309e718c..9a35197ad 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -185,8 +185,6 @@ en: removed: '%{user_name} removed %{labels}' sla: added: '%{user_name} added SLA policy %{sla_name}' - csat: - not_sent_due_to_messaging_window: 'CSAT survey not sent due to outgoing message restrictions' removed: '%{user_name} removed SLA policy %{sla_name}' muted: '%{user_name} has muted the conversation' unmuted: '%{user_name} has unmuted the conversation' diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index ad3446a13..aef91603d 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -435,20 +435,6 @@ RSpec.describe Conversation do end end - describe '#create_csat_not_sent_activity_message' do - subject(:create_csat_not_sent_activity_message) { conversation.create_csat_not_sent_activity_message } - - let(:conversation) { create(:conversation) } - - it 'creates CSAT not sent activity message' do - create_csat_not_sent_activity_message - expect(Conversations::ActivityMessageJob) - .to(have_been_enqueued.at_least(:once).with(conversation, { account_id: conversation.account_id, inbox_id: conversation.inbox_id, - message_type: :activity, - content: 'CSAT survey not sent due to outgoing message restrictions' })) - end - end - describe 'unread_messages' do subject(:unread_messages) { conversation.unread_messages } diff --git a/spec/services/message_templates/hook_execution_service_spec.rb b/spec/services/message_templates/hook_execution_service_spec.rb index 6e97aac42..24e40ea8d 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -121,9 +121,8 @@ describe MessageTemplates::HookExecutionService do create(:message, conversation: conversation, message_type: 'incoming') end - it 'calls ::MessageTemplates::Template::CsatSurvey when a conversation is resolved in an inbox with survey enabled and can reply' do + it 'calls ::MessageTemplates::Template::CsatSurvey when a conversation is resolved in an inbox with survey enabled' do conversation.inbox.update(csat_survey_enabled: true) - allow(conversation).to receive(:can_reply?).and_return(true) conversation.resolved! Conversations::ActivityMessageJob.perform_now(conversation, @@ -173,32 +172,6 @@ describe MessageTemplates::HookExecutionService do 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 cannot reply' do - conversation.inbox.update(csat_survey_enabled: true) - allow(conversation).to receive(:can_reply?).and_return(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 'creates activity message when CSAT not sent due to messaging window restriction' do - conversation.inbox.update(csat_survey_enabled: true) - allow(conversation).to receive(:can_reply?).and_return(false) - allow(conversation).to receive(:create_csat_not_sent_activity_message) - - 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(conversation).to have_received(:create_csat_not_sent_activity_message) - end end context 'when it is after working hours' do