From 3ce026e2bcb974258d8f5b36c88f38d1a0f84be6 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 28 May 2025 09:46:59 +0530 Subject: [PATCH 1/6] feat: save timezone from leadsquared API (#11583) --- .../mappers/conversation_mapper.rb | 22 ++-- .../crm/leadsquared/processor_service.rb | 4 +- app/services/crm/leadsquared/setup_service.rb | 3 +- config/integration/apps.yml | 1 + .../mappers/conversation_mapper_spec.rb | 100 +++++++++++++----- .../crm/leadsquared/processor_service_spec.rb | 4 +- .../crm/leadsquared/setup_service_spec.rb | 3 + 7 files changed, 93 insertions(+), 44 deletions(-) diff --git a/app/services/crm/leadsquared/mappers/conversation_mapper.rb b/app/services/crm/leadsquared/mappers/conversation_mapper.rb index 97a148435..c5c358cbf 100644 --- a/app/services/crm/leadsquared/mappers/conversation_mapper.rb +++ b/app/services/crm/leadsquared/mappers/conversation_mapper.rb @@ -6,17 +6,18 @@ class Crm::Leadsquared::Mappers::ConversationMapper # so this limits it ACTIVITY_NOTE_MAX_SIZE = 1800 - def self.map_conversation_activity(conversation) - new(conversation).conversation_activity + def self.map_conversation_activity(hook, conversation) + new(hook, conversation).conversation_activity end - def self.map_transcript_activity(conversation, messages = nil) - new(conversation, messages).transcript_activity + def self.map_transcript_activity(hook, conversation) + new(hook, conversation).transcript_activity end - def initialize(conversation, messages = nil) + def initialize(hook, conversation) + @hook = hook + @timezone = Time.find_zone(hook.settings['timezone']) || Time.zone @conversation = conversation - @messages = messages end def conversation_activity @@ -41,14 +42,14 @@ class Crm::Leadsquared::Mappers::ConversationMapper private - attr_reader :conversation, :messages + attr_reader :conversation def formatted_creation_time - conversation.created_at.strftime('%Y-%m-%d %H:%M:%S') + conversation.created_at.in_time_zone(@timezone).strftime('%Y-%m-%d %H:%M:%S') end def transcript_messages - @transcript_messages ||= messages || conversation.messages.chat.select(&:conversation_transcriptable?) + @transcript_messages ||= conversation.messages.chat.select(&:conversation_transcriptable?) end def format_messages @@ -77,8 +78,7 @@ class Crm::Leadsquared::Mappers::ConversationMapper end def message_time(message) - # TODO: Figure out what timezone to send the time in - message.created_at.strftime('%Y-%m-%d %H:%M') + message.created_at.in_time_zone(@timezone).strftime('%Y-%m-%d %H:%M') end def sender_name(message) diff --git a/app/services/crm/leadsquared/processor_service.rb b/app/services/crm/leadsquared/processor_service.rb index aedea51d7..ef33718f2 100644 --- a/app/services/crm/leadsquared/processor_service.rb +++ b/app/services/crm/leadsquared/processor_service.rb @@ -37,7 +37,7 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService activity_type: 'conversation', activity_code_key: 'conversation_activity_code', metadata_key: 'created_activity_id', - activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_conversation_activity(conversation) + activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_conversation_activity(@hook, conversation) ) end @@ -50,7 +50,7 @@ class Crm::Leadsquared::ProcessorService < Crm::BaseProcessorService activity_type: 'transcript', activity_code_key: 'transcript_activity_code', metadata_key: 'transcript_activity_id', - activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_transcript_activity(conversation) + activity_note: Crm::Leadsquared::Mappers::ConversationMapper.map_transcript_activity(@hook, conversation) ) end diff --git a/app/services/crm/leadsquared/setup_service.rb b/app/services/crm/leadsquared/setup_service.rb index 956ff1a10..0433f68fd 100644 --- a/app/services/crm/leadsquared/setup_service.rb +++ b/app/services/crm/leadsquared/setup_service.rb @@ -25,11 +25,12 @@ class Crm::Leadsquared::SetupService response = @client.get('Authentication.svc/UserByAccessKey.Get') endpoint_host = response['LSQCommonServiceURLs']['api'] app_host = response['LSQCommonServiceURLs']['app'] + timezone = response['TimeZone'] endpoint_url = "https://#{endpoint_host}/v2/" app_url = "https://#{app_host}/" - update_hook_settings({ :endpoint_url => endpoint_url, :app_url => app_url }) + update_hook_settings({ :endpoint_url => endpoint_url, :app_url => app_url, :timezone => timezone }) # replace the clients @client = Crm::Leadsquared::Api::BaseClient.new(@access_key, @secret_key, endpoint_url) diff --git a/config/integration/apps.yml b/config/integration/apps.yml index 10ba2e056..2921bf637 100644 --- a/config/integration/apps.yml +++ b/config/integration/apps.yml @@ -205,6 +205,7 @@ leadsquared: 'secret_key': { 'type': 'string' }, 'endpoint_url': { 'type': 'string' }, 'app_url': { 'type': 'string' }, + 'timezone': { 'type': 'string' }, 'enable_conversation_activity': { 'type': 'boolean' }, 'enable_transcript_activity': { 'type': 'boolean' }, 'conversation_activity_score': { 'type': 'string' }, diff --git a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb index 85bb08d74..0ddd4ac9f 100644 --- a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb +++ b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb @@ -6,15 +6,39 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do let(:conversation) { create(:conversation, account: account, inbox: inbox) } let(:user) { create(:user, name: 'John Doe') } let(:contact) { create(:contact, name: 'Jane Smith') } + let(:hook) do + create(:integrations_hook, :leadsquared, account: account, settings: { + 'access_key' => 'test_access_key', + 'secret_key' => 'test_secret_key', + 'endpoint_url' => 'https://api.leadsquared.com/v2', + 'timezone' => 'UTC' + }) + end + let(:hook_with_pst) do + create(:integrations_hook, :leadsquared, account: account, settings: { + 'access_key' => 'test_access_key', + 'secret_key' => 'test_secret_key', + 'endpoint_url' => 'https://api.leadsquared.com/v2', + 'timezone' => 'America/Los_Angeles' + }) + end + let(:hook_without_timezone) do + create(:integrations_hook, :leadsquared, account: account, settings: { + 'access_key' => 'test_access_key', + 'secret_key' => 'test_secret_key', + 'endpoint_url' => 'https://api.leadsquared.com/v2' + }) + end before do + account.enable_features('crm_integration') allow(GlobalConfig).to receive(:get).with('BRAND_NAME').and_return({ 'BRAND_NAME' => 'TestBrand' }) end describe '.map_conversation_activity' do - it 'generates conversation activity note' do - travel_to(Time.zone.parse('2024-01-01 10:00:00')) do - result = described_class.map_conversation_activity(conversation) + it 'generates conversation activity note with UTC timezone' do + travel_to(Time.zone.parse('2024-01-01 10:00:00 UTC')) do + result = described_class.map_conversation_activity(hook, conversation) expect(result).to include('New conversation started on TestBrand') expect(result).to include('Channel: Test Inbox') @@ -23,12 +47,29 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do expect(result).to include('View in TestBrand: http://') end end + + it 'formats time according to hook timezone setting' do + travel_to(Time.zone.parse('2024-01-01 18:00:00 UTC')) do + result = described_class.map_conversation_activity(hook_with_pst, conversation) + + # PST is UTC-8, so 18:00 UTC becomes 10:00:00 PST + expect(result).to include('Created: 2024-01-01 10:00:00') + end + end + + it 'falls back to system timezone when hook has no timezone setting' do + travel_to(Time.zone.parse('2024-01-01 10:00:00')) do + result = described_class.map_conversation_activity(hook_without_timezone, conversation) + + expect(result).to include('Created: 2024-01-01 10:00:00') + end + end end describe '.map_transcript_activity' do context 'when conversation has no messages' do it 'returns no messages message' do - result = described_class.map_transcript_activity(conversation) + result = described_class.map_transcript_activity(hook, conversation) expect(result).to eq('No messages in conversation') end end @@ -68,7 +109,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do end it 'generates transcript with messages in reverse chronological order' do - result = described_class.map_transcript_activity(conversation) + result = described_class.map_transcript_activity(hook, conversation) expect(result).to include('Conversation Transcript from TestBrand') expect(result).to include('Channel: Test Inbox') @@ -83,6 +124,22 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do expect(message_positions['[2024-01-01 10:01] Jane Smith: Hi there']).to be < message_positions['[2024-01-01 10:00] John Doe: Hello'] end + it 'formats message times according to hook timezone setting' do + travel_to(Time.zone.parse('2024-01-01 18:00:00 UTC')) do + create(:message, + conversation: conversation, + sender: user, + content: 'Test message', + message_type: :outgoing, + created_at: Time.zone.parse('2024-01-01 18:00:00 UTC')) + + result = described_class.map_transcript_activity(hook_with_pst, conversation) + + # PST is UTC-8, so 18:00 UTC becomes 10:00 PST + expect(result).to include('[2024-01-01 10:00] John Doe: Test message') + end + end + context 'when message has attachments' do let(:message_with_attachment) do create(:message, :with_attachment, @@ -96,7 +153,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do before { message_with_attachment } it 'includes attachment information' do - result = described_class.map_transcript_activity(conversation) + result = described_class.map_transcript_activity(hook, conversation) expect(result).to include('See attachment') expect(result).to include('[Attachment: image]') @@ -116,7 +173,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do before { empty_message } it 'shows no content placeholder' do - result = described_class.map_transcript_activity(conversation) + result = described_class.map_transcript_activity(hook, conversation) expect(result).to include('[No content]') end end @@ -134,25 +191,12 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do before { unnamed_sender_message } it 'uses sender type and id' do - result = described_class.map_transcript_activity(conversation) + result = described_class.map_transcript_activity(hook, conversation) expect(result).to include("User #{unnamed_sender_message.sender_id}") end end end - context 'when specific messages are provided' do - let(:message1) { create(:message, conversation: conversation, content: 'Message 1', message_type: :outgoing) } - let(:message2) { create(:message, conversation: conversation, content: 'Message 2', message_type: :outgoing) } - let(:specific_messages) { [message1] } - - it 'only includes provided messages' do - result = described_class.map_transcript_activity(conversation, specific_messages) - - expect(result).to include('Message 1') - expect(result).not_to include('Message 2') - end - end - context 'when messages exceed the ACTIVITY_NOTE_MAX_SIZE' do it 'truncates messages to stay within the character limit' do # Create a large number of messages with reasonably sized content @@ -169,7 +213,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do created_at: Time.zone.parse("2024-01-01 #{10 + i}:00:00")) end - result = described_class.map_transcript_activity(conversation, messages) + result = described_class.map_transcript_activity(hook, conversation) # Verify latest message is included (message 14) expect(result).to include("[2024-01-02 00:00] John Doe: #{long_message_content} 14") @@ -189,13 +233,13 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do it 'respects the ACTIVITY_NOTE_MAX_SIZE constant' do # Create a single message that would exceed the limit by itself giant_content = 'A' * 2000 - message = create(:message, - conversation: conversation, - sender: user, - content: giant_content, - message_type: :outgoing) + create(:message, + conversation: conversation, + sender: user, + content: giant_content, + message_type: :outgoing) - result = described_class.map_transcript_activity(conversation, [message]) + result = described_class.map_transcript_activity(hook, conversation) # Extract just the formatted messages part id = conversation.display_id diff --git a/spec/services/crm/leadsquared/processor_service_spec.rb b/spec/services/crm/leadsquared/processor_service_spec.rb index efdead00b..7008eb064 100644 --- a/spec/services/crm/leadsquared/processor_service_spec.rb +++ b/spec/services/crm/leadsquared/processor_service_spec.rb @@ -116,7 +116,7 @@ RSpec.describe Crm::Leadsquared::ProcessorService do before do allow(Crm::Leadsquared::Mappers::ConversationMapper).to receive(:map_conversation_activity) - .with(conversation) + .with(hook, conversation) .and_return(activity_note) end @@ -180,7 +180,7 @@ RSpec.describe Crm::Leadsquared::ProcessorService do before do allow(Crm::Leadsquared::Mappers::ConversationMapper).to receive(:map_transcript_activity) - .with(conversation) + .with(hook, conversation) .and_return(activity_note) end diff --git a/spec/services/crm/leadsquared/setup_service_spec.rb b/spec/services/crm/leadsquared/setup_service_spec.rb index 1d907ecda..8ebdc9691 100644 --- a/spec/services/crm/leadsquared/setup_service_spec.rb +++ b/spec/services/crm/leadsquared/setup_service_spec.rb @@ -8,6 +8,7 @@ RSpec.describe Crm::Leadsquared::SetupService do let(:activity_client) { instance_double(Crm::Leadsquared::Api::ActivityClient) } let(:endpoint_response) do { + 'TimeZone' => 'Asia/Kolkata', 'LSQCommonServiceURLs' => { 'api' => 'api-in.leadsquared.com', 'app' => 'app.leadsquared.com' @@ -45,6 +46,7 @@ RSpec.describe Crm::Leadsquared::SetupService do updated_settings = hook.reload.settings expect(updated_settings['endpoint_url']).to eq('https://api-in.leadsquared.com/v2/') expect(updated_settings['app_url']).to eq('https://app.leadsquared.com/') + expect(updated_settings['timezone']).to eq('Asia/Kolkata') expect(updated_settings['conversation_activity_code']).to eq(1001) expect(updated_settings['transcript_activity_code']).to eq(1002) end @@ -71,6 +73,7 @@ RSpec.describe Crm::Leadsquared::SetupService do updated_settings = hook.reload.settings expect(updated_settings['endpoint_url']).to eq('https://api-in.leadsquared.com/v2/') expect(updated_settings['app_url']).to eq('https://app.leadsquared.com/') + expect(updated_settings['timezone']).to eq('Asia/Kolkata') expect(updated_settings['conversation_activity_code']).to eq(1001) expect(updated_settings['transcript_activity_code']).to eq(1002) end From 443214e9a0ef1556737409f4508a9809333e1a60 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 28 May 2025 13:50:43 +0530 Subject: [PATCH 2/6] feat: add support for bunny CDN videos (#11601) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> --- lib/custom_markdown_renderer.rb | 10 +++++++++- lib/embed_renderer.rb | 15 +++++++++++++++ spec/lib/custom_markdown_renderer_spec.rb | 17 +++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/custom_markdown_renderer.rb b/lib/custom_markdown_renderer.rb index 75dc43700..902fc20a3 100644 --- a/lib/custom_markdown_renderer.rb +++ b/lib/custom_markdown_renderer.rb @@ -7,6 +7,7 @@ class CustomMarkdownRenderer < CommonMarker::HtmlRenderer MP4_REGEX = %r{https?://(?:www\.)?.+\.(mp4)} ARCADE_REGEX = %r{https?://(?:www\.)?app\.arcade\.software/share/([^&/]+)} WISTIA_REGEX = %r{https?://(?:www\.)?([^/]+)\.wistia\.com/medias/([^&/]+)} + BUNNY_REGEX = %r{https?://iframe\.mediadelivery\.net/play/(\d+)/([^&/?]+)} def text(node) content = node.string_content @@ -52,7 +53,8 @@ class CustomMarkdownRenderer < CommonMarker::HtmlRenderer MP4_REGEX => :make_video_embed, LOOM_REGEX => :make_loom_embed, ARCADE_REGEX => :make_arcade_embed, - WISTIA_REGEX => :make_wistia_embed + WISTIA_REGEX => :make_wistia_embed, + BUNNY_REGEX => :make_bunny_embed } embedding_methods.each do |regex, method| @@ -104,4 +106,10 @@ class CustomMarkdownRenderer < CommonMarker::HtmlRenderer video_id = arcade_match[1] EmbedRenderer.arcade(video_id) end + + def make_bunny_embed(bunny_match) + library_id = bunny_match[1] + video_id = bunny_match[2] + EmbedRenderer.bunny(library_id, video_id) + end end diff --git a/lib/embed_renderer.rb b/lib/embed_renderer.rb index 0a747bbb3..78f620376 100644 --- a/lib/embed_renderer.rb +++ b/lib/embed_renderer.rb @@ -84,4 +84,19 @@ module EmbedRenderer ) end + + def self.bunny(library_id, video_id) + %( +
+ +
+ ) + end end diff --git a/spec/lib/custom_markdown_renderer_spec.rb b/spec/lib/custom_markdown_renderer_spec.rb index 939965e91..23574e8c4 100644 --- a/spec/lib/custom_markdown_renderer_spec.rb +++ b/spec/lib/custom_markdown_renderer_spec.rb @@ -162,5 +162,22 @@ describe CustomMarkdownRenderer do expect(output).to include('src="https://www.youtube-nocookie.com/embed/VIDEO_ID"') end end + + context 'when link is a Bunny.net URL' do + let(:bunny_url) { 'https://iframe.mediadelivery.net/play/431789/1f105841-cad9-46fe-a70e-b7623c60797c' } + + it 'renders an iframe with Bunny embed code' do + output = render_markdown_link(bunny_url) + expect(output).to include('src="https://iframe.mediadelivery.net/embed/431789/1f105841-cad9-46fe-a70e-b7623c60797c?autoplay=false&loop=false&muted=false&preload=true&responsive=true"') + expect(output).to include('allowfullscreen') + expect(output).to include('allow="accelerometer; gyroscope; autoplay; encrypted-media; picture-in-picture;"') + end + + it 'wraps iframe in responsive container' do + output = render_markdown_link(bunny_url) + expect(output).to include('position: relative; padding-top: 56.25%;') + expect(output).to include('position: absolute; top: 0; height: 100%; width: 100%;') + end + end end end From b1120ae7fbfb69bf3f1ce68e4c47d9ebac71f08d Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 28 May 2025 13:50:50 +0530 Subject: [PATCH 3/6] feat: allow searching articles in omnisearch (#11558) Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> --- .../api/v1/accounts/search_controller.rb | 4 ++ app/javascript/dashboard/api/search.js | 9 +++ .../dashboard/i18n/locale/en/search.json | 6 +- .../components/SearchResultArticleItem.vue | 69 +++++++++++++++++++ .../components/SearchResultArticlesList.vue | 53 ++++++++++++++ .../modules/search/components/SearchView.vue | 56 +++++++++++++-- .../dashboard/modules/search/search.routes.js | 8 ++- .../store/modules/conversationSearch.js | 24 +++++++ .../specs/conversationSearch/actions.spec.js | 25 +++++++ .../specs/conversationSearch/getters.spec.js | 11 +++ .../conversationSearch/mutations.spec.js | 22 ++++++ .../dashboard/store/mutation-types.js | 2 + app/services/search_service.rb | 12 +++- .../v1/accounts/search/_article.json.jbuilder | 8 +++ .../_conversation_search_result.json.jbuilder | 15 ++++ .../v1/accounts/search/articles.json.jbuilder | 7 ++ .../v1/accounts/search/index.json.jbuilder | 22 ++---- config/routes.rb | 1 + .../api/v1/accounts/search_controller_spec.rb | 64 ++++++++++++++++- spec/services/search_service_spec.rb | 57 ++++++++++++++- 20 files changed, 449 insertions(+), 26 deletions(-) create mode 100644 app/javascript/dashboard/modules/search/components/SearchResultArticleItem.vue create mode 100644 app/javascript/dashboard/modules/search/components/SearchResultArticlesList.vue create mode 100644 app/views/api/v1/accounts/search/_article.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/_conversation_search_result.json.jbuilder create mode 100644 app/views/api/v1/accounts/search/articles.json.jbuilder diff --git a/app/controllers/api/v1/accounts/search_controller.rb b/app/controllers/api/v1/accounts/search_controller.rb index 35979f70f..13e3a6a6c 100644 --- a/app/controllers/api/v1/accounts/search_controller.rb +++ b/app/controllers/api/v1/accounts/search_controller.rb @@ -15,6 +15,10 @@ class Api::V1::Accounts::SearchController < Api::V1::Accounts::BaseController @result = search('Message') end + def articles + @result = search('Article') + end + private def search(search_type) diff --git a/app/javascript/dashboard/api/search.js b/app/javascript/dashboard/api/search.js index 7abb584c0..d533c2f28 100644 --- a/app/javascript/dashboard/api/search.js +++ b/app/javascript/dashboard/api/search.js @@ -40,6 +40,15 @@ class SearchAPI extends ApiClient { }, }); } + + articles({ q, page = 1 }) { + return axios.get(`${this.url}/articles`, { + params: { + q, + page: page, + }, + }); + } } export default new SearchAPI(); diff --git a/app/javascript/dashboard/i18n/locale/en/search.json b/app/javascript/dashboard/i18n/locale/en/search.json index 3cb566813..e8510ab97 100644 --- a/app/javascript/dashboard/i18n/locale/en/search.json +++ b/app/javascript/dashboard/i18n/locale/en/search.json @@ -4,12 +4,14 @@ "ALL": "All", "CONTACTS": "Contacts", "CONVERSATIONS": "Conversations", - "MESSAGES": "Messages" + "MESSAGES": "Messages", + "ARTICLES": "Articles" }, "SECTION": { "CONTACTS": "Contacts", "CONVERSATIONS": "Conversations", - "MESSAGES": "Messages" + "MESSAGES": "Messages", + "ARTICLES": "Articles" }, "VIEW_MORE": "View more", "LOAD_MORE": "Load more", diff --git a/app/javascript/dashboard/modules/search/components/SearchResultArticleItem.vue b/app/javascript/dashboard/modules/search/components/SearchResultArticleItem.vue new file mode 100644 index 000000000..7e2da950e --- /dev/null +++ b/app/javascript/dashboard/modules/search/components/SearchResultArticleItem.vue @@ -0,0 +1,69 @@ + + + diff --git a/app/javascript/dashboard/modules/search/components/SearchResultArticlesList.vue b/app/javascript/dashboard/modules/search/components/SearchResultArticlesList.vue new file mode 100644 index 000000000..679e411c2 --- /dev/null +++ b/app/javascript/dashboard/modules/search/components/SearchResultArticlesList.vue @@ -0,0 +1,53 @@ + + + diff --git a/app/javascript/dashboard/modules/search/components/SearchView.vue b/app/javascript/dashboard/modules/search/components/SearchView.vue index 1b0a9e4d7..bd48a3078 100644 --- a/app/javascript/dashboard/modules/search/components/SearchView.vue +++ b/app/javascript/dashboard/modules/search/components/SearchView.vue @@ -8,6 +8,7 @@ import { ROLES, CONVERSATION_PERMISSIONS, CONTACT_PERMISSIONS, + PORTAL_PERMISSIONS, } from 'dashboard/constants/permissions.js'; import { getUserPermissions, @@ -22,6 +23,7 @@ import SearchTabs from './SearchTabs.vue'; import SearchResultConversationsList from './SearchResultConversationsList.vue'; import SearchResultMessagesList from './SearchResultMessagesList.vue'; import SearchResultContactsList from './SearchResultContactsList.vue'; +import SearchResultArticlesList from './SearchResultArticlesList.vue'; const router = useRouter(); const store = useStore(); @@ -34,6 +36,7 @@ const pages = ref({ contacts: 1, conversations: 1, messages: 1, + articles: 1, }); const currentUser = useMapGetter('getCurrentUser'); @@ -43,6 +46,7 @@ const conversationRecords = useMapGetter( 'conversationSearch/getConversationRecords' ); const messageRecords = useMapGetter('conversationSearch/getMessageRecords'); +const articleRecords = useMapGetter('conversationSearch/getArticleRecords'); const uiFlags = useMapGetter('conversationSearch/getUIFlags'); const addTypeToRecords = (records, type) => @@ -57,6 +61,9 @@ const mappedConversations = computed(() => const mappedMessages = computed(() => addTypeToRecords(messageRecords, 'message') ); +const mappedArticles = computed(() => + addTypeToRecords(articleRecords, 'article') +); const isSelectedTabAll = computed(() => selectedTab.value === 'all'); @@ -66,6 +73,7 @@ const sliceRecordsIfAllTab = items => const contacts = computed(() => sliceRecordsIfAllTab(mappedContacts)); const conversations = computed(() => sliceRecordsIfAllTab(mappedConversations)); const messages = computed(() => sliceRecordsIfAllTab(mappedMessages)); +const articles = computed(() => sliceRecordsIfAllTab(mappedArticles)); const filterByTab = tab => computed(() => selectedTab.value === tab || isSelectedTabAll.value); @@ -73,6 +81,7 @@ const filterByTab = tab => const filterContacts = filterByTab('contacts'); const filterConversations = filterByTab('conversations'); const filterMessages = filterByTab('messages'); +const filterArticles = filterByTab('articles'); const userPermissions = computed(() => getUserPermissions(currentUser.value, currentAccountId.value) @@ -80,7 +89,12 @@ const userPermissions = computed(() => const TABS_CONFIG = { all: { - permissions: [CONTACT_PERMISSIONS, ...ROLES, ...CONVERSATION_PERMISSIONS], + permissions: [ + CONTACT_PERMISSIONS, + ...ROLES, + ...CONVERSATION_PERMISSIONS, + PORTAL_PERMISSIONS, + ], count: () => null, // No count for all tab }, contacts: { @@ -95,6 +109,10 @@ const TABS_CONFIG = { permissions: [...ROLES, ...CONVERSATION_PERMISSIONS], count: () => mappedMessages.value.length, }, + articles: { + permissions: [...ROLES, PORTAL_PERMISSIONS], + count: () => mappedArticles.value.length, + }, }; const tabs = computed(() => { @@ -123,6 +141,10 @@ const totalSearchResultsCount = computed(() => { permissions: [...ROLES, ...CONVERSATION_PERMISSIONS], count: () => conversations.value.length + messages.value.length, }, + articles: { + permissions: [...ROLES, PORTAL_PERMISSIONS], + count: () => articles.value.length, + }, }; return filterItemsByPermission( permissionCounts, @@ -138,12 +160,13 @@ const activeTabIndex = computed(() => { }); const isFetchingAny = computed(() => { - const { contact, message, conversation, isFetching } = uiFlags.value; + const { contact, message, conversation, article, isFetching } = uiFlags.value; return ( isFetching || contact.isFetching || message.isFetching || - conversation.isFetching + conversation.isFetching || + article.isFetching ); }); @@ -171,6 +194,7 @@ const showLoadMore = computed(() => { contacts: mappedContacts.value, conversations: mappedConversations.value, messages: mappedMessages.value, + articles: mappedArticles.value, }[selectedTab.value]; return ( @@ -185,10 +209,11 @@ const showViewMore = computed(() => ({ conversations: mappedConversations.value?.length > 5 && isSelectedTabAll.value, messages: mappedMessages.value?.length > 5 && isSelectedTabAll.value, + articles: mappedArticles.value?.length > 5 && isSelectedTabAll.value, })); const clearSearchResult = () => { - pages.value = { contacts: 1, conversations: 1, messages: 1 }; + pages.value = { contacts: 1, conversations: 1, messages: 1, articles: 1 }; store.dispatch('conversationSearch/clearSearchResults'); }; @@ -214,6 +239,7 @@ const loadMore = () => { contacts: 'conversationSearch/contactSearch', conversations: 'conversationSearch/conversationSearch', messages: 'conversationSearch/messageSearch', + articles: 'conversationSearch/articleSearch', }; if (uiFlags.value.isFetching || selectedTab.value === 'all') return; @@ -328,6 +354,28 @@ onUnmounted(() => { /> + + + + +
{ q: 'test', }); expect(dispatch).toHaveBeenCalledWith('messageSearch', { q: 'test' }); + expect(dispatch).toHaveBeenCalledWith('articleSearch', { q: 'test' }); }); }); @@ -150,6 +151,30 @@ describe('#actions', () => { }); }); + describe('#articleSearch', () => { + it('should handle successful article search', async () => { + axios.get.mockResolvedValue({ + data: { payload: { articles: [{ id: 1 }] } }, + }); + + await actions.articleSearch({ commit }, { q: 'test', page: 1 }); + expect(commit.mock.calls).toEqual([ + [types.ARTICLE_SEARCH_SET_UI_FLAG, { isFetching: true }], + [types.ARTICLE_SEARCH_SET, [{ id: 1 }]], + [types.ARTICLE_SEARCH_SET_UI_FLAG, { isFetching: false }], + ]); + }); + + it('should handle failed article search', async () => { + axios.get.mockRejectedValue({}); + await actions.articleSearch({ commit }, { q: 'test' }); + expect(commit.mock.calls).toEqual([ + [types.ARTICLE_SEARCH_SET_UI_FLAG, { isFetching: true }], + [types.ARTICLE_SEARCH_SET_UI_FLAG, { isFetching: false }], + ]); + }); + }); + describe('#clearSearchResults', () => { it('should commit clear search results mutation', () => { actions.clearSearchResults({ commit }); diff --git a/app/javascript/dashboard/store/modules/specs/conversationSearch/getters.spec.js b/app/javascript/dashboard/store/modules/specs/conversationSearch/getters.spec.js index ea3ca7048..efce6084a 100644 --- a/app/javascript/dashboard/store/modules/specs/conversationSearch/getters.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversationSearch/getters.spec.js @@ -37,6 +37,15 @@ describe('#getters', () => { ]); }); + it('getArticleRecords', () => { + const state = { + articleRecords: [{ id: 1, title: 'Article 1' }], + }; + expect(getters.getArticleRecords(state)).toEqual([ + { id: 1, title: 'Article 1' }, + ]); + }); + it('getUIFlags', () => { const state = { uiFlags: { @@ -45,6 +54,7 @@ describe('#getters', () => { contact: { isFetching: true }, message: { isFetching: false }, conversation: { isFetching: false }, + article: { isFetching: false }, }, }; expect(getters.getUIFlags(state)).toEqual({ @@ -53,6 +63,7 @@ describe('#getters', () => { contact: { isFetching: true }, message: { isFetching: false }, conversation: { isFetching: false }, + article: { isFetching: false }, }); }); }); diff --git a/app/javascript/dashboard/store/modules/specs/conversationSearch/mutations.spec.js b/app/javascript/dashboard/store/modules/specs/conversationSearch/mutations.spec.js index 7bef2e527..bf7e833d0 100644 --- a/app/javascript/dashboard/store/modules/specs/conversationSearch/mutations.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversationSearch/mutations.spec.js @@ -101,17 +101,39 @@ describe('#mutations', () => { }); }); + describe('#ARTICLE_SEARCH_SET', () => { + it('should append new article records to existing ones', () => { + const state = { articleRecords: [{ id: 1 }] }; + mutations[types.ARTICLE_SEARCH_SET](state, [{ id: 2 }]); + expect(state.articleRecords).toEqual([{ id: 1 }, { id: 2 }]); + }); + }); + + describe('#ARTICLE_SEARCH_SET_UI_FLAG', () => { + it('set article search UI flags correctly', () => { + const state = { + uiFlags: { + article: { isFetching: true }, + }, + }; + mutations[types.ARTICLE_SEARCH_SET_UI_FLAG](state, { isFetching: false }); + expect(state.uiFlags.article).toEqual({ isFetching: false }); + }); + }); + describe('#CLEAR_SEARCH_RESULTS', () => { it('should clear all search records', () => { const state = { contactRecords: [{ id: 1 }], conversationRecords: [{ id: 1 }], messageRecords: [{ id: 1 }], + articleRecords: [{ id: 1 }], }; mutations[types.CLEAR_SEARCH_RESULTS](state); expect(state.contactRecords).toEqual([]); expect(state.conversationRecords).toEqual([]); expect(state.messageRecords).toEqual([]); + expect(state.articleRecords).toEqual([]); }); }); }); diff --git a/app/javascript/dashboard/store/mutation-types.js b/app/javascript/dashboard/store/mutation-types.js index a74207e92..f3817c45a 100644 --- a/app/javascript/dashboard/store/mutation-types.js +++ b/app/javascript/dashboard/store/mutation-types.js @@ -317,8 +317,10 @@ export default { CONVERSATION_SEARCH_SET: 'CONVERSATION_SEARCH_SET', CONVERSATION_SEARCH_SET_UI_FLAG: 'CONVERSATION_SEARCH_SET_UI_FLAG', MESSAGE_SEARCH_SET: 'MESSAGE_SEARCH_SET', + ARTICLE_SEARCH_SET: 'ARTICLE_SEARCH_SET', CLEAR_SEARCH_RESULTS: 'CLEAR_SEARCH_RESULTS', MESSAGE_SEARCH_SET_UI_FLAG: 'MESSAGE_SEARCH_SET_UI_FLAG', + ARTICLE_SEARCH_SET_UI_FLAG: 'ARTICLE_SEARCH_SET_UI_FLAG', FULL_SEARCH_SET_UI_FLAG: 'FULL_SEARCH_SET_UI_FLAG', SET_CONVERSATION_PARTICIPANTS_UI_FLAG: 'SET_CONVERSATION_PARTICIPANTS_UI_FLAG', diff --git a/app/services/search_service.rb b/app/services/search_service.rb index 5999c88a6..40d862b19 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -9,8 +9,10 @@ class SearchService { conversations: filter_conversations } when 'Contact' { contacts: filter_contacts } + when 'Article' + { articles: filter_articles } else - { contacts: filter_contacts, messages: filter_messages, conversations: filter_conversations } + { contacts: filter_contacts, messages: filter_messages, conversations: filter_conversations, articles: filter_articles } end end @@ -90,4 +92,12 @@ class SearchService ILIKE :search OR identifier ILIKE :search", search: "%#{search_query}%" ).resolved_contacts.order_on_last_activity_at('desc').page(params[:page]).per(15) end + + def filter_articles + @articles = current_account.articles + .text_search(search_query) + .reorder('updated_at DESC') + .page(params[:page]) + .per(15) + end end diff --git a/app/views/api/v1/accounts/search/_article.json.jbuilder b/app/views/api/v1/accounts/search/_article.json.jbuilder new file mode 100644 index 000000000..a3cf94614 --- /dev/null +++ b/app/views/api/v1/accounts/search/_article.json.jbuilder @@ -0,0 +1,8 @@ +json.id article.id +json.title article.title +json.locale article.locale +json.content article.content +json.slug article.slug +json.portal_slug article.portal.slug +json.account_id article.account_id +json.category_name article.category&.name diff --git a/app/views/api/v1/accounts/search/_conversation_search_result.json.jbuilder b/app/views/api/v1/accounts/search/_conversation_search_result.json.jbuilder new file mode 100644 index 000000000..a0b7e0203 --- /dev/null +++ b/app/views/api/v1/accounts/search/_conversation_search_result.json.jbuilder @@ -0,0 +1,15 @@ +json.id conversation.display_id +json.account_id conversation.account_id +json.created_at conversation.created_at.to_i +json.message do + json.partial! 'message', formats: [:json], message: conversation.messages.try(:first) +end +json.contact do + json.partial! 'contact', formats: [:json], contact: conversation.contact if conversation.try(:contact).present? +end +json.inbox do + json.partial! 'inbox', formats: [:json], inbox: conversation.inbox if conversation.try(:inbox).present? +end +json.agent do + json.partial! 'agent', formats: [:json], agent: conversation.assignee if conversation.try(:assignee).present? +end diff --git a/app/views/api/v1/accounts/search/articles.json.jbuilder b/app/views/api/v1/accounts/search/articles.json.jbuilder new file mode 100644 index 000000000..7d4fe031c --- /dev/null +++ b/app/views/api/v1/accounts/search/articles.json.jbuilder @@ -0,0 +1,7 @@ +json.payload do + json.articles do + json.array! @result[:articles] do |article| + json.partial! 'article', formats: [:json], article: article + end + end +end \ No newline at end of file diff --git a/app/views/api/v1/accounts/search/index.json.jbuilder b/app/views/api/v1/accounts/search/index.json.jbuilder index 1c6e86284..a3d8f1858 100644 --- a/app/views/api/v1/accounts/search/index.json.jbuilder +++ b/app/views/api/v1/accounts/search/index.json.jbuilder @@ -1,21 +1,7 @@ json.payload do json.conversations do json.array! @result[:conversations] do |conversation| - json.id conversation.display_id - json.account_id conversation.account_id - json.created_at conversation.created_at.to_i - json.message do - json.partial! 'message', formats: [:json], message: conversation.messages.try(:first) - end - json.contact do - json.partial! 'contact', formats: [:json], contact: conversation.contact if conversation.try(:contact).present? - end - json.inbox do - json.partial! 'inbox', formats: [:json], inbox: conversation.inbox if conversation.try(:inbox).present? - end - json.agent do - json.partial! 'agent', formats: [:json], agent: conversation.assignee if conversation.try(:assignee).present? - end + json.partial! 'conversation_search_result', formats: [:json], conversation: conversation end end json.contacts do @@ -23,10 +9,14 @@ json.payload do json.partial! 'contact', formats: [:json], contact: contact end end - json.messages do json.array! @result[:messages] do |message| json.partial! 'message', formats: [:json], message: message end end + json.articles do + json.array! @result[:articles] do |article| + json.partial! 'article', formats: [:json], article: article + end + end end diff --git a/config/routes.rb b/config/routes.rb index 4b4db7b6d..d1705d605 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -137,6 +137,7 @@ Rails.application.routes.draw do get :conversations get :messages get :contacts + get :articles end end diff --git a/spec/controllers/api/v1/accounts/search_controller_spec.rb b/spec/controllers/api/v1/accounts/search_controller_spec.rb index b5644cebf..ea59bec9c 100644 --- a/spec/controllers/api/v1/accounts/search_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/search_controller_spec.rb @@ -11,6 +11,11 @@ RSpec.describe 'Search', type: :request do create(:message, conversation: conversation, account: account, content: 'test2') create(:contact_inbox, contact_id: contact.id, inbox_id: conversation.inbox.id) create(:inbox_member, user: agent, inbox: conversation.inbox) + + # Create articles for testing + portal = create(:portal, account: account) + create(:article, title: 'Test Article Guide', content: 'This is a test article content', + account: account, portal: portal, author: agent, status: 'published') end describe 'GET /api/v1/accounts/{account.id}/search' do @@ -33,10 +38,11 @@ RSpec.describe 'Search', type: :request do response_data = JSON.parse(response.body, symbolize_names: true) expect(response_data[:payload][:messages].first[:content]).to eq 'test2' - expect(response_data[:payload].keys).to contain_exactly(:contacts, :conversations, :messages) + expect(response_data[:payload].keys).to contain_exactly(:contacts, :conversations, :messages, :articles) expect(response_data[:payload][:messages].length).to eq 2 expect(response_data[:payload][:conversations].length).to eq 1 expect(response_data[:payload][:contacts].length).to eq 1 + expect(response_data[:payload][:articles].length).to eq 1 end end end @@ -115,4 +121,60 @@ RSpec.describe 'Search', type: :request do end end end + + describe 'GET /api/v1/accounts/{account.id}/search/articles' do + context 'when it is an unauthenticated user' do + it 'returns unauthorized' do + get "/api/v1/accounts/#{account.id}/search/articles", params: { q: 'test' } + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when it is an authenticated user' do + it 'returns all articles containing the search query' do + get "/api/v1/accounts/#{account.id}/search/articles", + headers: agent.create_new_auth_token, + params: { q: 'test' }, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + + expect(response_data[:payload].keys).to contain_exactly(:articles) + expect(response_data[:payload][:articles].length).to eq 1 + expect(response_data[:payload][:articles].first[:title]).to eq 'Test Article Guide' + end + + it 'returns empty results when no articles match the search query' do + get "/api/v1/accounts/#{account.id}/search/articles", + headers: agent.create_new_auth_token, + params: { q: 'nonexistent' }, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + + expect(response_data[:payload].keys).to contain_exactly(:articles) + expect(response_data[:payload][:articles].length).to eq 0 + end + + it 'supports pagination' do + portal = create(:portal, account: account) + 16.times do |i| + create(:article, title: "Test Article #{i}", account: account, portal: portal, author: agent, status: 'published') + end + + get "/api/v1/accounts/#{account.id}/search/articles", + headers: agent.create_new_auth_token, + params: { q: 'test', page: 1 }, + as: :json + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true) + + expect(response_data[:payload][:articles].length).to eq 15 # Default per_page is 15 + end + end + end end diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb index af097a2c9..22809d042 100644 --- a/spec/services/search_service_spec.rb +++ b/spec/services/search_service_spec.rb @@ -10,6 +10,11 @@ describe SearchService do let!(:harry) { create(:contact, name: 'Harry Potter', email: 'test@test.com', account_id: account.id) } let!(:conversation) { create(:conversation, contact: harry, inbox: inbox, account: account) } let!(:message) { create(:message, account: account, inbox: inbox, content: 'Harry Potter is a wizard') } + let!(:portal) { create(:portal, account: account) } + let(:article) do + create(:article, title: 'Harry Potter Magic Guide', content: 'Learn about wizardry', account: account, portal: portal, author: user, + status: 'published') + end before do create(:inbox_member, user: user, inbox: inbox) @@ -27,7 +32,7 @@ describe SearchService do it 'returns all for all' do search_type = 'all' search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) - expect(search.perform.keys).to match_array(%i[contacts messages conversations]) + expect(search.perform.keys).to match_array(%i[contacts messages conversations articles]) end it 'returns contacts for contacts' do @@ -47,6 +52,12 @@ describe SearchService do search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) expect(search.perform.keys).to match_array(%i[conversations]) end + + it 'returns articles for articles' do + search_type = 'Article' + search = described_class.new(current_user: user, current_account: account, params: params, search_type: search_type) + expect(search.perform.keys).to match_array(%i[articles]) + end end context 'when contact search' do @@ -143,6 +154,50 @@ describe SearchService do expect(search.perform[:conversations].map(&:id)).to include new_converstion.id end end + + context 'when article search' do + it 'orders results by updated_at desc' do + # Create articles with explicit timestamps + older_time = 2.days.ago + newer_time = 1.hour.ago + + article2 = create(:article, title: 'Spellcasting Guide', + account: account, portal: portal, author: user, status: 'published') + # rubocop:disable Rails/SkipsModelValidations + article2.update_column(:updated_at, older_time) + # rubocop:enable Rails/SkipsModelValidations + + article3 = create(:article, title: 'Spellcasting Manual', + account: account, portal: portal, author: user, status: 'published') + # rubocop:disable Rails/SkipsModelValidations + article3.update_column(:updated_at, newer_time) + # rubocop:enable Rails/SkipsModelValidations + + params = { q: 'Spellcasting' } + search = described_class.new(current_user: user, current_account: account, params: params, search_type: 'Article') + results = search.perform[:articles] + + # Check the timestamps to understand ordering + results.map { |a| [a.id, a.updated_at] } + + # Should be ordered by updated_at desc (newer first) + expect(results.length).to eq(2) + expect(results.first.updated_at).to be > results.second.updated_at + end + + it 'returns paginated results' do + # Create many articles to test pagination + 16.times do |i| + create(:article, title: "Magic Article #{i}", account: account, portal: portal, author: user, status: 'published') + end + + params = { q: 'Magic', page: 1 } + search = described_class.new(current_user: user, current_account: account, params: params, search_type: 'Article') + results = search.perform[:articles] + + expect(results.length).to eq(15) # Default per_page is 15 + end + end end describe '#use_gin_search' do From dc335e88c9dee4f6cfd60f41bb99f39bcf37c3a6 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Wed, 28 May 2025 15:15:05 +0530 Subject: [PATCH 4/6] fix: External links in widget not opening in new tab (#11608) --- app/javascript/portal/portalHelpers.js | 11 +-- app/javascript/portal/specs/portal.spec.js | 98 +++++++++++++++++++++- 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/app/javascript/portal/portalHelpers.js b/app/javascript/portal/portalHelpers.js index 5cced0fa4..9cb28e63a 100644 --- a/app/javascript/portal/portalHelpers.js +++ b/app/javascript/portal/portalHelpers.js @@ -38,16 +38,9 @@ export const openExternalLinksInNewTab = () => { document.addEventListener('click', event => { if (!isOnArticlePage) return; - // Some of the links come wrapped in strong tag through prosemirror - - const isTagAnchor = event.target.tagName === 'A'; - const isParentTagAnchor = - event.target.tagName === 'STRONG' && - event.target.parentNode.tagName === 'A'; - - if (isTagAnchor || isParentTagAnchor) { - const link = isTagAnchor ? event.target : event.target.parentNode; + const link = event.target.closest('a'); + if (link) { const isInternalLink = link.hostname === window.location.hostname || link.href.includes(customDomain) || diff --git a/app/javascript/portal/specs/portal.spec.js b/app/javascript/portal/specs/portal.spec.js index 13edd3718..861950a57 100644 --- a/app/javascript/portal/specs/portal.spec.js +++ b/app/javascript/portal/specs/portal.spec.js @@ -1,6 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { JSDOM } from 'jsdom'; -import { InitializationHelpers } from '../portalHelpers'; +import { + InitializationHelpers, + openExternalLinksInNewTab, +} from '../portalHelpers'; describe('InitializationHelpers.navigateToLocalePage', () => { let dom; @@ -44,3 +47,96 @@ describe('InitializationHelpers.navigateToLocalePage', () => { ); }); }); + +describe('openExternalLinksInNewTab', () => { + let dom; + let document; + let window; + + beforeEach(() => { + dom = new JSDOM( + ` + + +
+ External + Internal + Custom + CodeBold + +
+ + `, + { url: 'https://app.chatwoot.com/hc/article' } + ); + + document = dom.window.document; + window = dom.window; + + window.portalConfig = { + customDomain: 'custom.domain.com', + hostURL: 'app.chatwoot.com', + }; + + global.document = document; + global.window = window; + }); + + afterEach(() => { + dom = null; + document = null; + window = null; + delete global.document; + delete global.window; + }); + + const simulateClick = selector => { + const element = document.querySelector(selector); + const event = new window.MouseEvent('click', { bubbles: true }); + element.dispatchEvent(event); + return element.closest('a') || element; + }; + + it('opens external links in new tab', () => { + openExternalLinksInNewTab(); + + const link = simulateClick('#external'); + + expect(link.target).toBe('_blank'); + expect(link.rel).toBe('noopener noreferrer'); + }); + + it('preserves internal links', () => { + openExternalLinksInNewTab(); + + const internal = simulateClick('#internal'); + const custom = simulateClick('#custom'); + + expect(internal.target).not.toBe('_blank'); + expect(custom.target).not.toBe('_blank'); + }); + + it('handles clicks on nested elements', () => { + openExternalLinksInNewTab(); + + simulateClick('#nested code'); + simulateClick('#nested strong'); + + const link = document.getElementById('nested'); + expect(link.target).toBe('_blank'); + expect(link.rel).toBe('noopener noreferrer'); + }); + + it('handles links inside list items with strong tags', () => { + openExternalLinksInNewTab(); + + // Click on the strong element inside the link in the list + simulateClick('#list-link strong'); + + const link = document.getElementById('list-link'); + expect(link.target).toBe('_blank'); + expect(link.rel).toBe('noopener noreferrer'); + }); +}); From f916fb2924e525b879ac060526bd3ccfb80ef47e Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 28 May 2025 17:56:32 +0530 Subject: [PATCH 5/6] fix: handle empty customDomain when checking for `isInternalLink` (#11609) This PR improves the portal's internal link detection logic to be more robust when handling empty or undefined configuration values. Previously, the code could fail when `customDomain` was empty, causing external links to incorrectly behave as internal links. The fix introduces a new `isSameOrigin` helper function that safely compares URLs using proper URL parsing and origin comparison, gracefully handling edge cases like missing domains, relative paths, and malformed URLs. This ensures external links consistently open in new tabs regardless of portal configuration completeness. --- app/javascript/portal/portalHelpers.js | 63 ++++++++++++++++++---- app/javascript/portal/specs/portal.spec.js | 45 +++++++++++++++- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/app/javascript/portal/portalHelpers.js b/app/javascript/portal/portalHelpers.js index 9cb28e63a..68e2cb6a8 100644 --- a/app/javascript/portal/portalHelpers.js +++ b/app/javascript/portal/portalHelpers.js @@ -25,15 +25,56 @@ export const getHeadingsfromTheArticle = () => { return rows; }; +/** + * Converts various input formats to URL objects. + * Handles URL objects, domain strings, relative paths, and full URLs. + * @param {string|URL} input - Input to convert to URL object + * @returns {URL|null} URL object or null if input is invalid + */ +const toURL = input => { + if (!input) return null; + if (input instanceof URL) return input; + + if ( + typeof input === 'string' && + !input.includes('://') && + !input.startsWith('/') + ) { + return new URL(`https://${input}`); + } + + if (typeof input === 'string' && input.startsWith('/')) { + return new URL(input, window.location.origin); + } + + return new URL(input); +}; + +/** + * Determines if two URLs belong to the same host by comparing their normalized URL objects. + * Handles various input formats including URL objects, domain strings, relative paths, and full URLs. + * Returns false if either URL cannot be parsed or normalized. + * @param {string|URL} url1 - First URL to compare + * @param {string|URL} url2 - Second URL to compare + * @returns {boolean} True if both URLs have the same host, false otherwise + */ +const isSameHost = (url1, url2) => { + try { + const urlObj1 = toURL(url1); + const urlObj2 = toURL(url2); + + if (!urlObj1 || !urlObj2) return false; + + return urlObj1.hostname === urlObj2.hostname; + } catch (error) { + return false; + } +}; + export const openExternalLinksInNewTab = () => { const { customDomain, hostURL } = window.portalConfig; - const isSameHost = - window.location.href.includes(customDomain) || - window.location.href.includes(hostURL); - - // Modify external links only on articles page const isOnArticlePage = - isSameHost && document.querySelector('#cw-article-content') !== null; + document.querySelector('#cw-article-content') !== null; document.addEventListener('click', event => { if (!isOnArticlePage) return; @@ -41,10 +82,14 @@ export const openExternalLinksInNewTab = () => { const link = event.target.closest('a'); if (link) { + const currentLocation = window.location.href; + const linkHref = link.href; + + // Check against current location and custom domains const isInternalLink = - link.hostname === window.location.hostname || - link.href.includes(customDomain) || - link.href.includes(hostURL); + isSameHost(linkHref, currentLocation) || + (customDomain && isSameHost(linkHref, customDomain)) || + (hostURL && isSameHost(linkHref, hostURL)); if (!isInternalLink) { link.target = '_blank'; diff --git a/app/javascript/portal/specs/portal.spec.js b/app/javascript/portal/specs/portal.spec.js index 861950a57..5205c5d45 100644 --- a/app/javascript/portal/specs/portal.spec.js +++ b/app/javascript/portal/specs/portal.spec.js @@ -103,7 +103,6 @@ describe('openExternalLinksInNewTab', () => { openExternalLinksInNewTab(); const link = simulateClick('#external'); - expect(link.target).toBe('_blank'); expect(link.rel).toBe('noopener noreferrer'); }); @@ -139,4 +138,48 @@ describe('openExternalLinksInNewTab', () => { expect(link.target).toBe('_blank'); expect(link.rel).toBe('noopener noreferrer'); }); + + it('opens external links in a new tab even if customDomain is empty', () => { + window = dom.window; + window.portalConfig = { + hostURL: 'app.chatwoot.com', + }; + + global.window = window; + + openExternalLinksInNewTab(); + + const link = simulateClick('#external'); + const internal = simulateClick('#internal'); + const custom = simulateClick('#custom'); + + expect(link.target).toBe('_blank'); + expect(link.rel).toBe('noopener noreferrer'); + + expect(internal.target).not.toBe('_blank'); + // this will be blank since the configs customDomain is empty + // which is a fair expectation + expect(custom.target).toBe('_blank'); + }); + + it('opens external links in a new tab even if hostURL is empty', () => { + window = dom.window; + window.portalConfig = { + customDomain: 'custom.domain.com', + }; + + global.window = window; + + openExternalLinksInNewTab(); + + const link = simulateClick('#external'); + const internal = simulateClick('#internal'); + const custom = simulateClick('#custom'); + + expect(link.target).toBe('_blank'); + expect(link.rel).toBe('noopener noreferrer'); + + expect(internal.target).not.toBe('_blank'); + expect(custom.target).not.toBe('_blank'); + }); }); From b5ebc4763723e15ad3a04ab0b0ecb1e01087a2b1 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 28 May 2025 19:34:11 +0530 Subject: [PATCH 6/6] fix: Send CSAT survey only when agent can reply in conversation (#11584) Fixes https://github.com/chatwoot/chatwoot/issues/11569 ## Problem On platforms like WhatsApp and Facebook Messenger, customers cannot reply to messages after 24 hours (or other channel-specific messaging windows). Despite this limitation, the system continued sending CSAT surveys to customers outside their messaging window, making it impossible for them to respond. ## Solution Added a check for `conversation.can_reply?` in the `should_send_csat_survey?` method. This leverages the existing `MessageWindowService` which already handles all channel-specific messaging window logic. --- .../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, 71 insertions(+), 6 deletions(-) create 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 54e58b4d9..0d6741c7a 100644 --- a/app/models/concerns/activity_message_handler.rb +++ b/app/models/concerns/activity_message_handler.rb @@ -5,6 +5,7 @@ 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 new file mode 100644 index 000000000..7a2488c4d --- /dev/null +++ b/app/models/concerns/csat_activity_message_handler.rb @@ -0,0 +1,8 @@ +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 a8a4c4318..8291c8a9c 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? - ::MessageTemplates::Template::CsatSurvey.new(conversation: conversation).perform if should_send_csat_survey? + handle_csat_survey end def should_send_out_of_office_message? @@ -65,13 +65,26 @@ class MessageTemplates::HookExecutionService true end - def should_send_csat_survey? + def handle_csat_survey return unless csat_enabled_conversation? - # only send CSAT once in a conversation - return if conversation.messages.where(content_type: :input_csat).present? + return if csat_already_sent? - true + # 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? end end MessageTemplates::HookExecutionService.prepend_mod_with('MessageTemplates::HookExecutionService') diff --git a/config/locales/en.yml b/config/locales/en.yml index 9a35197ad..b309e718c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -185,6 +185,8 @@ 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 aef91603d..ad3446a13 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -435,6 +435,20 @@ 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 24e40ea8d..6e97aac42 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -121,8 +121,9 @@ 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' do + it 'calls ::MessageTemplates::Template::CsatSurvey when a conversation is resolved in an inbox with survey enabled and can reply' do conversation.inbox.update(csat_survey_enabled: true) + allow(conversation).to receive(:can_reply?).and_return(true) conversation.resolved! Conversations::ActivityMessageJob.perform_now(conversation, @@ -172,6 +173,32 @@ 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