From 896473f03ed479a332acd53e88a474b5997c320f Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Wed, 17 Jan 2024 16:44:37 +0530 Subject: [PATCH 01/18] fix: Notification count is incorrect when the number of notifications exceeds the page size. (#8723) - The notification count is incorrect when the number of notifications exceeds the page size. --- app/controllers/api/v1/accounts/notifications_controller.rb | 2 +- .../dashboard/notifications/components/NotificationsView.vue | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v1/accounts/notifications_controller.rb b/app/controllers/api/v1/accounts/notifications_controller.rb index 9cea3fbb4..a3d8df27e 100644 --- a/app/controllers/api/v1/accounts/notifications_controller.rb +++ b/app/controllers/api/v1/accounts/notifications_controller.rb @@ -9,7 +9,7 @@ class Api::V1::Accounts::NotificationsController < Api::V1::Accounts::BaseContro def index @unread_count = notification_finder.unread_count @notifications = notification_finder.perform - @count = @notifications.count + @count = notification_finder.count end def read_all diff --git a/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationsView.vue b/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationsView.vue index 39a1c6b58..058305a7a 100644 --- a/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationsView.vue +++ b/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationsView.vue @@ -11,6 +11,7 @@ From eb972684b315ab1e4cb92dac95d2cbf906db3e9f Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Thu, 18 Jan 2024 09:41:53 +0530 Subject: [PATCH 02/18] feat: Show notification last active time instead of created time (#8724) --- .../notifications/components/NotificationPanelItem.vue | 2 +- .../dashboard/notifications/components/NotificationTable.vue | 2 +- app/views/api/v1/accounts/notifications/index.json.jbuilder | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationPanelItem.vue b/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationPanelItem.vue index 262114816..3a4fa0c86 100644 --- a/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationPanelItem.vue +++ b/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationPanelItem.vue @@ -56,7 +56,7 @@ - {{ dynamicTime(notificationItem.created_at) }} + {{ dynamicTime(notificationItem.last_activity_at) }} diff --git a/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationTable.vue b/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationTable.vue index 650a2b942..3cc4dde62 100644 --- a/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationTable.vue +++ b/app/javascript/dashboard/routes/dashboard/notifications/components/NotificationTable.vue @@ -58,7 +58,7 @@
- {{ dynamicTime(notificationItem.created_at) }} + {{ dynamicTime(notificationItem.last_activity_at) }}
diff --git a/app/views/api/v1/accounts/notifications/index.json.jbuilder b/app/views/api/v1/accounts/notifications/index.json.jbuilder index e04ce22cf..b46dcb9ec 100644 --- a/app/views/api/v1/accounts/notifications/index.json.jbuilder +++ b/app/views/api/v1/accounts/notifications/index.json.jbuilder @@ -19,6 +19,7 @@ json.data do json.secondary_actor notification.secondary_actor&.push_event_data json.user notification.user.push_event_data json.created_at notification.created_at.to_i + json.last_activity_at notification.last_activity_at.to_i end end end From c899cc825d25a7d39c5a957a7e4deabc8f7d23f3 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Thu, 18 Jan 2024 13:05:58 +0530 Subject: [PATCH 03/18] fix: Handle Contact import `MalformedCSVError` (#8706) --- app/jobs/data_import_job.rb | 17 +++++++++++++++-- .../channel_notifications_mailer.rb | 9 +++++++++ .../contact_import_failed.liquid | 3 +++ spec/jobs/data_import_job_spec.rb | 15 +++++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 app/views/mailers/administrator_notifications/channel_notifications_mailer/contact_import_failed.liquid diff --git a/app/jobs/data_import_job.rb b/app/jobs/data_import_job.rb index 0a973c8bd..9703d2e50 100644 --- a/app/jobs/data_import_job.rb +++ b/app/jobs/data_import_job.rb @@ -8,8 +8,12 @@ class DataImportJob < ApplicationJob def perform(data_import) @data_import = data_import @contact_manager = DataImport::ContactManager.new(@data_import.account) - process_import_file - send_import_notification_to_admin + begin + process_import_file + send_import_notification_to_admin + rescue CSV::MalformedCSVError => e + handle_csv_error(e) + end end private @@ -83,7 +87,16 @@ class DataImportJob < ApplicationJob end end + def handle_csv_error(error) # rubocop:disable Lint/UnusedMethodArgument + @data_import.update!(status: :failed) + send_import_failed_notification_to_admin + end + def send_import_notification_to_admin AdministratorNotifications::ChannelNotificationsMailer.with(account: @data_import.account).contact_import_complete(@data_import).deliver_later end + + def send_import_failed_notification_to_admin + AdministratorNotifications::ChannelNotificationsMailer.with(account: @data_import.account).contact_import_failed.deliver_later + end end diff --git a/app/mailers/administrator_notifications/channel_notifications_mailer.rb b/app/mailers/administrator_notifications/channel_notifications_mailer.rb index b25a6d66b..8c52a9bd3 100644 --- a/app/mailers/administrator_notifications/channel_notifications_mailer.rb +++ b/app/mailers/administrator_notifications/channel_notifications_mailer.rb @@ -51,6 +51,15 @@ class AdministratorNotifications::ChannelNotificationsMailer < ApplicationMailer send_mail_with_liquid(to: admin_emails, subject: subject) and return end + def contact_import_failed + return unless smtp_config_set_or_development? + + subject = 'Contact Import Failed' + + @meta = {} + send_mail_with_liquid(to: admin_emails, subject: subject) and return + end + def contact_export_complete(file_url) return unless smtp_config_set_or_development? diff --git a/app/views/mailers/administrator_notifications/channel_notifications_mailer/contact_import_failed.liquid b/app/views/mailers/administrator_notifications/channel_notifications_mailer/contact_import_failed.liquid new file mode 100644 index 000000000..835cff258 --- /dev/null +++ b/app/views/mailers/administrator_notifications/channel_notifications_mailer/contact_import_failed.liquid @@ -0,0 +1,3 @@ +

Hello,

+ +

Your contact import has failed. It appears that the CSV file you uploaded may not be valid. We kindly request that you review the file and ensure it complies with the required format.

diff --git a/spec/jobs/data_import_job_spec.rb b/spec/jobs/data_import_job_spec.rb index ca20ce43b..5ece07e53 100644 --- a/spec/jobs/data_import_job_spec.rb +++ b/spec/jobs/data_import_job_spec.rb @@ -151,5 +151,20 @@ RSpec.describe DataImportJob do end end end + + context 'when the CSV file is invalid' do + let(:invalid_csv_content) do + "id,name,email,phone_number,company\n1,\"Clarice Uzzell,\"missing_quote,918080808080,Acmecorp\n2,Marieann Creegan,,+918080808081,Acmecorp" + end + + before do + allow(data_import.import_file).to receive(:download).and_return(invalid_csv_content) + end + + it 'does not import any data and handles the MalformedCSVError' do + expect { described_class.perform_now(data_import) } + .to change { data_import.reload.status }.from('pending').to('failed') + end + end end end From fdbb3bf4b1d25d664c602acdd1cc3133caf66efc Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Wed, 17 Jan 2024 23:45:16 -0800 Subject: [PATCH 04/18] fix: Optimize email fetching logic to fix bandwidth exceeded issues (#8730) The Inboxes::FetchImapEmailsJob is designed to fetch the entire email object and check if its message id is already in the database. However, this process is resource-intensive and time-consuming, as fetching the full email object takes a significant amount of time. On average, fetching 100 emails can take approximately 3-4 minutes to complete depending on the IMAP server. Since we are not using server flags to identify which emails have already been fetched (to avoid compatibility issues with flags in different email services), we have to fetch all previously available emails. Currently we fetch all the messages that were created from yesterday. This becomes problematic with services like Gmail, which throttle requests based on bandwidth usage. To address this issue, I have updated the logic as follows: Fetch the sequence IDs of all the mails in the "Inbox" that were created from yesterday. Use the FETCH command to fetch only the message-ids using BODY.PEEK[HEADER.FIELDS (MESSAGE-ID)] with the sequence IDs we got in the previous step. This is a faster operation with lower bandwidth usage, as it only returns the sequence ID and message ID. Check if the message IDs are already present in the database for the selected inbox. If not present, fetch the entire email object using the FETCH command and create the message. If the message ID is already present, ignore it and move on to the next message-id. I have also addressed the issue of duplicate emails appearing in conversations when two fetch email jobs occur simultaneously. I have added a lock for the channel to ensure that the job gracefully exits without waiting for the current job to complete. Fixes #7247 Fixes #6082 Fixes #8314 Co-authored-by: Sojan --- app/jobs/inboxes/fetch_imap_emails_job.rb | 66 ++++++++++++----------- lib/redis/redis_keys.rb | 1 + 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/app/jobs/inboxes/fetch_imap_emails_job.rb b/app/jobs/inboxes/fetch_imap_emails_job.rb index 8d3582440..b4cd640e9 100644 --- a/app/jobs/inboxes/fetch_imap_emails_job.rb +++ b/app/jobs/inboxes/fetch_imap_emails_job.rb @@ -1,17 +1,21 @@ require 'net/imap' -class Inboxes::FetchImapEmailsJob < ApplicationJob +class Inboxes::FetchImapEmailsJob < MutexApplicationJob queue_as :scheduled_jobs def perform(channel) return unless should_fetch_email?(channel) - process_email_for_channel(channel) + with_lock(::Redis::Alfred::EMAIL_MESSAGE_MUTEX, inbox_id: channel.inbox.id) do + process_email_for_channel(channel) + end rescue *ExceptionList::IMAP_EXCEPTIONS => e Rails.logger.error e channel.authorization_error! rescue EOFError, OpenSSL::SSL::SSLError, Net::IMAP::NoResponseError, Net::IMAP::BadResponseError => e Rails.logger.error e + rescue LockAcquisitionError + Rails.logger.error "Lock failed for #{inbox.id}" rescue StandardError => e ChatwootExceptionTracker.new(e, account: channel.account).capture_exception end @@ -35,33 +39,43 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob def fetch_mail_for_channel(channel) imap_inbox = authenticated_imap_inbox(channel, channel.imap_password, 'PLAIN') - last_email_time = DateTime.parse(Net::IMAP.format_datetime(last_email_time(channel))) - received_mails(imap_inbox).each do |message_id| - inbound_mail = Mail.read_from_string imap_inbox.fetch(message_id, 'RFC822')[0].attr['RFC822'] + fetch_message_ids(imap_inbox, channel).each do |message_id_uid_pair| + uid, message_id = message_id_uid_pair + next if email_already_present?(channel, message_id) - mail_info_logger(channel, inbound_mail, message_id) - - next if email_already_present?(channel, inbound_mail, last_email_time) + inbound_mail = Mail.read_from_string(imap_inbox.fetch(uid, 'RFC822')[0].attr['RFC822']) + mail_info_logger(channel, inbound_mail, uid) process_mail(inbound_mail, channel) end end - def email_already_present?(channel, inbound_mail, _last_email_time) - channel.inbox.messages.find_by(source_id: inbound_mail.message_id).present? + def fetch_message_ids(imap_inbox, channel) + uids = fetch_uids(imap_inbox) + Rails.logger.info "FETCH_EMAILS_FROM: #{channel.email} - Found #{uids.length} emails \n\n\n\n" + + message_ids = [] + uids.each_slice(10).each do |batch| + uid_fetched = imap_inbox.fetch(batch, 'BODY.PEEK[HEADER.FIELDS (MESSAGE-ID)]') + # print uid_fetched + uid_fetched.each do |data| + message_id = data.attr['BODY[HEADER.FIELDS (MESSAGE-ID)]'].to_s.scan(/<(.+?)>/).flatten.first + message_ids.push([data.seqno, message_id]) + end + end + + message_ids end - def received_mails(imap_inbox) + def email_already_present?(channel, message_id) + channel.inbox.messages.find_by(source_id: message_id).present? + end + + def fetch_uids(imap_inbox) imap_inbox.search(['BEFORE', tomorrow, 'SINCE', yesterday]) end - def processed_email?(current_email, last_email_time) - return current_email.date < last_email_time if current_email.date.present? - - false - end - def fetch_mail_for_ms_provider(channel) return if channel.provider_config['access_token'].blank? @@ -75,7 +89,7 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob end def process_mails(imap_inbox, channel) - received_mails(imap_inbox).each do |message_id| + fetch_uids(imap_inbox).each do |message_id| inbound_mail = Mail.read_from_string imap_inbox.fetch(message_id, 'RFC822')[0].attr['RFC822'] mail_info_logger(channel, inbound_mail, message_id) @@ -86,11 +100,11 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob end end - def mail_info_logger(channel, inbound_mail, message_id) + def mail_info_logger(channel, inbound_mail, uid) return if Rails.env.test? Rails.logger.info(" - #{channel.provider} Email id: #{inbound_mail.from} and message_source_id: #{inbound_mail.message_id}, message_id: #{message_id}") + #{channel.provider} Email id: #{inbound_mail.from} - message_source_id: #{inbound_mail.message_id} - sequence id: #{uid}") end def authenticated_imap_inbox(channel, access_token, auth_method) @@ -100,18 +114,6 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob imap end - def last_email_time(channel) - # we are only checking for emails in last 2 day - last_email_incoming_message = channel.inbox.messages.incoming.where('messages.created_at >= ?', 2.days.ago).last - if last_email_incoming_message.present? - time = last_email_incoming_message.content_attributes['email']['date'] - time ||= last_email_incoming_message.created_at.to_s - end - time ||= 1.hour.ago.to_s - - DateTime.parse(time) - end - def yesterday (Time.zone.today - 1).strftime('%d-%b-%Y') end diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index 01c1fc0b5..535d3774d 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -38,4 +38,5 @@ module Redis::RedisKeys FACEBOOK_MESSAGE_MUTEX = 'FB_MESSAGE_CREATE_LOCK::%s::%s'.freeze IG_MESSAGE_MUTEX = 'IG_MESSAGE_CREATE_LOCK::%s::%s'.freeze SLACK_MESSAGE_MUTEX = 'SLACK_MESSAGE_LOCK::%s::%s'.freeze + EMAIL_MESSAGE_MUTEX = 'EMAIL_CHANNEL_LOCK::%s'.freeze end From 1f4d860d9d832ef8d390f59eeab83028af991755 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Thu, 18 Jan 2024 00:46:25 -0800 Subject: [PATCH 05/18] fix: Use channel.inbox instead of inbox (#8734) --- app/jobs/inboxes/fetch_imap_emails_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/inboxes/fetch_imap_emails_job.rb b/app/jobs/inboxes/fetch_imap_emails_job.rb index b4cd640e9..d42e5969b 100644 --- a/app/jobs/inboxes/fetch_imap_emails_job.rb +++ b/app/jobs/inboxes/fetch_imap_emails_job.rb @@ -15,7 +15,7 @@ class Inboxes::FetchImapEmailsJob < MutexApplicationJob rescue EOFError, OpenSSL::SSL::SSLError, Net::IMAP::NoResponseError, Net::IMAP::BadResponseError => e Rails.logger.error e rescue LockAcquisitionError - Rails.logger.error "Lock failed for #{inbox.id}" + Rails.logger.error "Lock failed for #{channel.inbox.id}" rescue StandardError => e ChatwootExceptionTracker.new(e, account: channel.account).capture_exception end From 4bf23adcf5d38e4958b6b37fd6b4e1a48c620153 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 18 Jan 2024 16:06:13 +0530 Subject: [PATCH 06/18] feat: use `short_summary` for downloading reports [CW-2962] (#8733) --- app/builders/v2/report_builder.rb | 8 ++++++++ app/helpers/api/v2/accounts/reports_helper.rb | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/builders/v2/report_builder.rb b/app/builders/v2/report_builder.rb index 1442c5799..2725c55a3 100644 --- a/app/builders/v2/report_builder.rb +++ b/app/builders/v2/report_builder.rb @@ -46,6 +46,14 @@ class V2::ReportBuilder } end + def short_summary + { + conversations_count: conversations.count, + avg_first_response_time: avg_first_response_time_summary, + avg_resolution_time: avg_resolution_time_summary + } + end + def conversation_metrics if params[:type].equal?(:account) live_conversations diff --git a/app/helpers/api/v2/accounts/reports_helper.rb b/app/helpers/api/v2/accounts/reports_helper.rb index 0604eba2f..3d87154fd 100644 --- a/app/helpers/api/v2/accounts/reports_helper.rb +++ b/app/helpers/api/v2/accounts/reports_helper.rb @@ -37,7 +37,7 @@ module Api::V2::Accounts::ReportsHelper business_hours: ActiveModel::Type::Boolean.new.cast(params[:business_hours]) } ) - ).summary + ).short_summary end private From 5f6e17f3071bb1238d5ac04f8bf7d62fd256a23c Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Thu, 18 Jan 2024 15:36:36 +0400 Subject: [PATCH 07/18] feat: Use Telegram HTML Parsemode (#8731) - this ensures that the markdown formatted messages from the Chatwoot dashboard will render consistently in telegram UI for the supported types like bold, italics, links etc --- app/models/channel/telegram.rb | 28 +++++++----- spec/models/channel/telegram_spec.rb | 64 +++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/app/models/channel/telegram.rb b/app/models/channel/telegram.rb index 365ca59c8..f6b97f2d2 100644 --- a/app/models/channel/telegram.rb +++ b/app/models/channel/telegram.rb @@ -149,24 +149,32 @@ class Channel::Telegram < ApplicationRecord }) end - def convert_markdown_to_telegram(text) - ## supported characters : https://core.telegram.org/bots/api#markdown-style - ## To implement MarkdownV2, we will need to do a lot of escaping + def convert_markdown_to_telegram_html(text) + # ref: https://core.telegram.org/bots/api#html-style - # Convert bold - double asterisks to single asterisk in Telegram - # Chatwoot uses double asterisks for bold, while telegram used single asterisk - text.gsub!(/\*\*(.*?)\*\*/, '*\1*') - text + # escape html tags in text. We are subbing \n to
since commonmark will strip exta '\n' + text = CGI.escapeHTML(text.gsub("\n", '
')) + + # convert markdown to html + html = CommonMarker.render_html(text).strip + + # remove all html tags except b, strong, i, em, u, ins, s, strike, del, a, code, pre, blockquote + stripped_html = Rails::HTML5::SafeListSanitizer.new.sanitize(html, tags: %w[b strong i em u ins s strike del a code pre blockquote], + attributes: %w[href]) + + # converted escaped br tags to \n + stripped_html.gsub('<br>', "\n") end def message_request(chat_id, text, reply_markup = nil, reply_to_message_id = nil) - text_to_md = convert_markdown_to_telegram(text) + text_payload = convert_markdown_to_telegram_html(text) + HTTParty.post("#{telegram_api_url}/sendMessage", body: { chat_id: chat_id, - text: text_to_md, + text: text_payload, reply_markup: reply_markup, - parse_mode: 'Markdown', + parse_mode: 'HTML', reply_to_message_id: reply_to_message_id }) end diff --git a/spec/models/channel/telegram_spec.rb b/spec/models/channel/telegram_spec.rb index 248e7b43e..2cc927977 100644 --- a/spec/models/channel/telegram_spec.rb +++ b/spec/models/channel/telegram_spec.rb @@ -3,6 +3,58 @@ require 'rails_helper' RSpec.describe Channel::Telegram do let(:telegram_channel) { create(:channel_telegram) } + describe '#convert_markdown_to_telegram_html' do + subject { telegram_channel.send(:convert_markdown_to_telegram_html, text) } + + context 'when text contains multiple newline characters' do + let(:text) { "Line one\nLine two\n\nLine four" } + + it 'preserves multiple newline characters' do + expect(subject).to eq("Line one\nLine two\n\nLine four") + end + end + + context 'when text contains broken markdown' do + let(:text) { 'This is a **broken markdown with HTML tags.' } + + it 'does not break and properly converts to Telegram HTML format and escapes html tags' do + expect(subject).to eq('This is a **broken markdown with <b>HTML</b> tags.') + end + end + + context 'when text contains markdown and HTML elements' do + let(:text) { "Hello *world*! This is bold and this is italic.\nThis is a new line." } + + it 'converts markdown to Telegram HTML format and escapes other html' do + expect(subject).to eq("Hello world! This is <b>bold</b> and this is <i>italic</i>.\nThis is a new line.") + end + end + + context 'when text contains unsupported HTML tags' do + let(:text) { 'This is a test with unsupported tags.' } + + it 'removes unsupported HTML tags' do + expect(subject).to eq('This is a <span>test</span> with unsupported tags.') + end + end + + context 'when text contains special characters' do + let(:text) { 'Special characters: & < >' } + + it 'escapes special characters' do + expect(subject).to eq('Special characters: & < >') + end + end + + context 'when text contains markdown links' do + let(:text) { 'Check this [link](http://example.com) out!' } + + it 'converts markdown links to Telegram HTML format' do + expect(subject).to eq('Check this link out!') + end + end + end + context 'when a valid message and empty attachments' do it 'send message' do message = create(:message, message_type: :outgoing, content: 'test', @@ -10,7 +62,7 @@ RSpec.describe Channel::Telegram do stub_request(:post, "https://api.telegram.org/bot#{telegram_channel.bot_token}/sendMessage") .with( - body: 'chat_id=123&text=test&reply_markup=&parse_mode=Markdown&reply_to_message_id=' + body: 'chat_id=123&text=test&reply_markup=&parse_mode=HTML&reply_to_message_id=' ) .to_return( status: 200, @@ -21,13 +73,15 @@ RSpec.describe Channel::Telegram do expect(telegram_channel.send_message_on_telegram(message)).to eq('telegram_123') end - it 'send message with markdown converted to telegram markdown' do + it 'send message with markdown converted to telegram HTML' do message = create(:message, message_type: :outgoing, content: '**test** *test* ~test~', conversation: create(:conversation, inbox: telegram_channel.inbox, additional_attributes: { 'chat_id' => '123' })) stub_request(:post, "https://api.telegram.org/bot#{telegram_channel.bot_token}/sendMessage") .with( - body: "chat_id=123&text=#{ERB::Util.url_encode('*test* *test* ~test~')}&reply_markup=&parse_mode=Markdown&reply_to_message_id=" + body: "chat_id=123&text=#{ + ERB::Util.url_encode('test test ~test~') + }&reply_markup=&parse_mode=HTML&reply_to_message_id=" ) .to_return( status: 200, @@ -49,7 +103,7 @@ RSpec.describe Channel::Telegram do .with( body: 'chat_id=123&text=test' \ '&reply_markup=%7B%22one_time_keyboard%22%3Atrue%2C%22inline_keyboard%22%3A%5B%5B%7B%22text%22%3A%22test%22%2C%22' \ - 'callback_data%22%3A%22test%22%7D%5D%5D%7D&parse_mode=Markdown&reply_to_message_id=' + 'callback_data%22%3A%22test%22%7D%5D%5D%7D&parse_mode=HTML&reply_to_message_id=' ) .to_return( status: 200, @@ -66,7 +120,7 @@ RSpec.describe Channel::Telegram do stub_request(:post, "https://api.telegram.org/bot#{telegram_channel.bot_token}/sendMessage") .with( - body: 'chat_id=123&text=test&reply_markup=&parse_mode=Markdown&reply_to_message_id=' + body: 'chat_id=123&text=test&reply_markup=&parse_mode=HTML&reply_to_message_id=' ) .to_return( status: 403, From e39c14460b860d5e3d23d989dd6af48404ad1bb4 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 18 Jan 2024 17:48:30 +0530 Subject: [PATCH 08/18] fix: validate url for Dashboard Apps [CW-2979] (#8736) --- app/models/dashboard_app.rb | 5 +++- .../dashboard_apps_controller_spec.rb | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/app/models/dashboard_app.rb b/app/models/dashboard_app.rb index 7316dd344..e8a7edd4c 100644 --- a/app/models/dashboard_app.rb +++ b/app/models/dashboard_app.rb @@ -33,9 +33,12 @@ class DashboardApp < ApplicationRecord 'required' => %w[url type], 'properties' => { 'type' => { 'enum': ['frame'] }, - 'url' => { :type => 'string', 'format' => 'uri' } + 'url' => { '$ref' => '#/definitions/saneUrl' } } }, + 'definitions' => { + 'saneUrl' => { 'format' => 'uri', 'pattern' => '^https?://' } + }, 'additionalProperties' => false, 'minItems' => 1 } diff --git a/spec/controllers/api/v1/accounts/dashboard_apps_controller_spec.rb b/spec/controllers/api/v1/accounts/dashboard_apps_controller_spec.rb index e545b0e35..100f914bb 100644 --- a/spec/controllers/api/v1/accounts/dashboard_apps_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/dashboard_apps_controller_spec.rb @@ -55,8 +55,12 @@ RSpec.describe 'DashboardAppsController', type: :request do describe 'POST /api/v1/accounts/{account.id}/dashboard_apps' do let(:payload) { { dashboard_app: { title: 'CRM Dashboard', content: [{ type: 'frame', url: 'https://link.com' }] } } } + let(:no_ssl_payload) { { dashboard_app: { title: 'CRM Dashboard', content: [{ type: 'frame', url: 'http://link.com' }] } } } let(:invalid_type_payload) { { dashboard_app: { title: 'CRM Dashboard', content: [{ type: 'dda', url: 'https://link.com' }] } } } let(:invalid_url_payload) { { dashboard_app: { title: 'CRM Dashboard', content: [{ type: 'frame', url: 'com' }] } } } + let(:non_http_url_payload) do + { dashboard_app: { title: 'CRM Dashboard', content: [{ type: 'frame', url: 'ftp://wontwork.chatwoot.com/hello-world' }] } } + end context 'when it is an unauthenticated user' do it 'returns unauthorized' do @@ -82,6 +86,19 @@ RSpec.describe 'DashboardAppsController', type: :request do expect(json_response['content'][0]['type']).to eq payload[:dashboard_app][:content][0][:type] end + it 'creates the dashboard app even if the URL does not have SSL' do + expect do + post "/api/v1/accounts/#{account.id}/dashboard_apps", headers: user.create_new_auth_token, + params: no_ssl_payload + end.to change(DashboardApp, :count).by(1) + + expect(response).to have_http_status(:success) + json_response = response.parsed_body + expect(json_response['title']).to eq 'CRM Dashboard' + expect(json_response['content'][0]['link']).to eq payload[:dashboard_app][:content][0][:link] + expect(json_response['content'][0]['type']).to eq payload[:dashboard_app][:content][0][:type] + end + it 'does not create the dashboard app if invalid URL' do expect do post "/api/v1/accounts/#{account.id}/dashboard_apps", headers: user.create_new_auth_token, @@ -93,6 +110,17 @@ RSpec.describe 'DashboardAppsController', type: :request do expect(json_response['message']).to eq 'Content : Invalid data' end + it 'does not create the dashboard app if non HTTP URL' do + expect do + post "/api/v1/accounts/#{account.id}/dashboard_apps", headers: user.create_new_auth_token, + params: non_http_url_payload + end.not_to change(DashboardApp, :count) + + expect(response).to have_http_status(:unprocessable_entity) + json_response = response.parsed_body + expect(json_response['message']).to eq 'Content : Invalid data' + end + it 'does not create the dashboard app if invalid type' do expect do post "/api/v1/accounts/#{account.id}/dashboard_apps", headers: user.create_new_auth_token, From aacf326ca1921442561d53c0f592fe859713affe Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 18 Jan 2024 17:49:27 +0530 Subject: [PATCH 09/18] refactor: remove exception tracker (#8737) --- app/helpers/api/v1/inboxes_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/api/v1/inboxes_helper.rb b/app/helpers/api/v1/inboxes_helper.rb index 57b750274..d734a346e 100644 --- a/app/helpers/api/v1/inboxes_helper.rb +++ b/app/helpers/api/v1/inboxes_helper.rb @@ -53,7 +53,7 @@ module Api::V1::InboxesHelper rescue StandardError => e raise StandardError, e.message ensure - ChatwootExceptionTracker.new(e).capture_exception if e.present? + Rails.logger.error "[Api::V1::InboxesHelper] check_imap_connection failed with #{e.message}" if e.present? end def check_smtp_connection(channel_data, smtp) From ce8190dacf5d63eee50a62249ef8450699d20cf8 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Thu, 18 Jan 2024 18:49:32 +0400 Subject: [PATCH 10/18] fix: API error when using SuperAdmin token (#8739) - Fixes the issue in release 3.5.0, which causes SuperAdmin tokens to throw error during API calls Fixes: #8719 --- .../concerns/access_token_auth_helper.rb | 9 ++++++++- spec/controllers/api/base_controller_spec.rb | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/access_token_auth_helper.rb b/app/controllers/concerns/access_token_auth_helper.rb index c35a28d7d..0a6c89860 100644 --- a/app/controllers/concerns/access_token_auth_helper.rb +++ b/app/controllers/concerns/access_token_auth_helper.rb @@ -14,7 +14,14 @@ module AccessTokenAuthHelper render_unauthorized('Invalid Access Token') && return if @access_token.blank? @resource = @access_token.owner - Current.user = @resource if [User, AgentBot].include?(@resource.class) + Current.user = @resource if allowed_current_user_type?(@resource) + end + + def allowed_current_user_type?(resource) + return true if resource.is_a?(User) + return true if resource.is_a?(AgentBot) + + false end def validate_bot_access_token! diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index c0230240e..ac2306d14 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -29,6 +29,25 @@ RSpec.describe 'API Base', type: :request do end end + describe 'request with api_access_token for a super admin' do + before do + user.update!(type: 'SuperAdmin') + end + + context 'when its a valid api_access_token' do + it 'returns current user information' do + get '/api/v1/profile', + headers: { api_access_token: user.access_token.token }, + as: :json + + expect(response).to have_http_status(:success) + json_response = response.parsed_body + expect(json_response['id']).to eq(user.id) + expect(json_response['email']).to eq(user.email) + end + end + end + describe 'request with api_access_token for bot' do let!(:agent_bot) { create(:agent_bot) } let!(:inbox) { create(:inbox, account: account) } From 19474e00743fc96a830891f0bb220f31d468c8fc Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Thu, 18 Jan 2024 20:57:18 +0530 Subject: [PATCH 11/18] chore: Improve active job error logs for deserialization error (#8742) - Improve active job error logs for deserialization error Co-authored-by: Sojan --- app/jobs/application_job.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index c577b1908..22be5f036 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -1,6 +1,8 @@ class ApplicationJob < ActiveJob::Base # https://api.rubyonrails.org/v5.2.1/classes/ActiveJob/Exceptions/ClassMethods.html - discard_on ActiveJob::DeserializationError do |_job, error| - Rails.logger.error("Skipping job because of ActiveJob::DeserializationError (#{error.message})") + discard_on ActiveJob::DeserializationError do |job, error| + Rails.logger.info("Skipping #{job.class} with #{ + job.instance_variable_get(:@serialized_arguments) + } because of ActiveJob::DeserializationError (#{error.message})") end end From 0ac015ce7a26b8a7f1b09367c198fd3bdc936a98 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Fri, 19 Jan 2024 13:03:23 +0530 Subject: [PATCH 12/18] feat: Add a job to clear notifications that were created before 1 month (#8732) --- .../remove_old_notification_job.rb | 8 +++++++ app/jobs/trigger_scheduled_items_job.rb | 3 +++ .../remove_old_notification_job_spec.rb | 24 +++++++++++++++++++ spec/jobs/trigger_scheduled_items_job_spec.rb | 5 ++++ 4 files changed, 40 insertions(+) create mode 100644 app/jobs/notification/remove_old_notification_job.rb create mode 100644 spec/jobs/notification/remove_old_notification_job_spec.rb diff --git a/app/jobs/notification/remove_old_notification_job.rb b/app/jobs/notification/remove_old_notification_job.rb new file mode 100644 index 000000000..770461744 --- /dev/null +++ b/app/jobs/notification/remove_old_notification_job.rb @@ -0,0 +1,8 @@ +class Notification::RemoveOldNotificationJob < ApplicationJob + queue_as :low + + def perform + Notification.where('created_at < ?', 1.month.ago) + .find_each(batch_size: 1000, &:delete) + end +end diff --git a/app/jobs/trigger_scheduled_items_job.rb b/app/jobs/trigger_scheduled_items_job.rb index 9fc7a6491..82634e4d5 100644 --- a/app/jobs/trigger_scheduled_items_job.rb +++ b/app/jobs/trigger_scheduled_items_job.rb @@ -19,5 +19,8 @@ class TriggerScheduledItemsJob < ApplicationJob # Job to sync whatsapp templates Channels::Whatsapp::TemplatesSyncSchedulerJob.perform_later + + # Job to clear notifications which are older than 1 month + Notification::RemoveOldNotificationJob.perform_later end end diff --git a/spec/jobs/notification/remove_old_notification_job_spec.rb b/spec/jobs/notification/remove_old_notification_job_spec.rb new file mode 100644 index 000000000..efc6616eb --- /dev/null +++ b/spec/jobs/notification/remove_old_notification_job_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe Notification::RemoveOldNotificationJob do + let(:user) { create(:user) } + let(:conversation) { create(:conversation) } + + it 'enqueues the job' do + notification = create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation) + expect do + described_class.perform_later(notification) + end.to have_enqueued_job(described_class) + .on_queue('low') + end + + it 'removes old notifications which are older than 1 month' do + create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation, created_at: 2.months.ago) + create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation, created_at: 1.month.ago) + create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation, created_at: 1.day.ago) + create(:notification, user: user, notification_type: 'conversation_creation', primary_actor: conversation, created_at: 1.hour.ago) + + described_class.perform_now + expect(Notification.count).to eq(2) + end +end diff --git a/spec/jobs/trigger_scheduled_items_job_spec.rb b/spec/jobs/trigger_scheduled_items_job_spec.rb index 9fd846c2d..c3f34dfeb 100644 --- a/spec/jobs/trigger_scheduled_items_job_spec.rb +++ b/spec/jobs/trigger_scheduled_items_job_spec.rb @@ -40,5 +40,10 @@ RSpec.describe TriggerScheduledItemsJob do expect(Channels::Whatsapp::TemplatesSyncSchedulerJob).to receive(:perform_later).once described_class.perform_now end + + it 'triggers Notification::RemoveOldNotificationJob' do + expect(Notification::RemoveOldNotificationJob).to receive(:perform_later).once + described_class.perform_now + end end end From c29a9ad0627e91dc6727d42dd115551a55f9431d Mon Sep 17 00:00:00 2001 From: Nithin David Thomas <1277421+nithindavid@users.noreply.github.com> Date: Fri, 19 Jan 2024 01:37:24 -0800 Subject: [PATCH 13/18] feat: Updates branding logo and name in public portal footer (#8745) - Use white-label settings for Chatwoot Help Center Co-authored-by: Sojan --- .../public/api/v1/portals/base_controller.rb | 5 +++++ app/views/icons/_chatwoot-logo.html.erb | 11 ----------- app/views/public/api/v1/portals/_footer.html.erb | 12 ++++++++++-- 3 files changed, 15 insertions(+), 13 deletions(-) delete mode 100644 app/views/icons/_chatwoot-logo.html.erb diff --git a/app/controllers/public/api/v1/portals/base_controller.rb b/app/controllers/public/api/v1/portals/base_controller.rb index e003a62ba..4d3cc56b8 100644 --- a/app/controllers/public/api/v1/portals/base_controller.rb +++ b/app/controllers/public/api/v1/portals/base_controller.rb @@ -1,6 +1,7 @@ class Public::Api::V1::Portals::BaseController < PublicController before_action :show_plain_layout before_action :set_color_scheme + before_action :set_global_config around_action :set_locale after_action :allow_iframe_requests @@ -60,4 +61,8 @@ class Public::Api::V1::Portals::BaseController < PublicController portal render 'public/api/v1/portals/error/404', status: :not_found end + + def set_global_config + @global_config = GlobalConfig.get('LOGO_THUMBNAIL', 'BRAND_NAME', 'BRAND_URL') + end end diff --git a/app/views/icons/_chatwoot-logo.html.erb b/app/views/icons/_chatwoot-logo.html.erb deleted file mode 100644 index 5377a0a02..000000000 --- a/app/views/icons/_chatwoot-logo.html.erb +++ /dev/null @@ -1,11 +0,0 @@ - - - woot-log - Created with Sketch. - - \ No newline at end of file diff --git a/app/views/public/api/v1/portals/_footer.html.erb b/app/views/public/api/v1/portals/_footer.html.erb index 162f0d9b6..8de87b8f2 100644 --- a/app/views/public/api/v1/portals/_footer.html.erb +++ b/app/views/public/api/v1/portals/_footer.html.erb @@ -2,9 +2,17 @@