From 46ec92c86ea8f8d36272e5b31e2f5a1bb04d1a65 Mon Sep 17 00:00:00 2001 From: Pranav Date: Fri, 14 Mar 2025 17:37:36 -0700 Subject: [PATCH 01/16] fix: Improve performance of most hit APIs in widget (#11089) - Cache campaigns for better performance - Fix N+1 queries in inbox members - Remove unused related articles --- .../api/v1/widget/campaigns_controller.rb | 6 ++++- .../api/v1/widget/inbox_members_controller.rb | 2 +- .../widget/store/modules/campaign.js | 21 ++++++++++----- .../modules/specs/campaign/actions.spec.js | 26 +++++++++++++++++-- .../modules/specs/campaign/mutations.spec.js | 3 ++- .../api/v1/models/_category.json.jbuilder | 20 -------------- 6 files changed, 47 insertions(+), 31 deletions(-) diff --git a/app/controllers/api/v1/widget/campaigns_controller.rb b/app/controllers/api/v1/widget/campaigns_controller.rb index cb9b96a38..10a73aa62 100644 --- a/app/controllers/api/v1/widget/campaigns_controller.rb +++ b/app/controllers/api/v1/widget/campaigns_controller.rb @@ -2,6 +2,10 @@ class Api::V1::Widget::CampaignsController < Api::V1::Widget::BaseController skip_before_action :set_contact def index - @campaigns = @web_widget.inbox.campaigns.where(enabled: true) + @campaigns = @web_widget + .inbox + .campaigns + .where(enabled: true, account_id: @web_widget.inbox.account_id) + .includes(:sender) end end diff --git a/app/controllers/api/v1/widget/inbox_members_controller.rb b/app/controllers/api/v1/widget/inbox_members_controller.rb index c4bc377ea..22934388b 100644 --- a/app/controllers/api/v1/widget/inbox_members_controller.rb +++ b/app/controllers/api/v1/widget/inbox_members_controller.rb @@ -2,6 +2,6 @@ class Api::V1::Widget::InboxMembersController < Api::V1::Widget::BaseController skip_before_action :set_contact def index - @inbox_members = @web_widget.inbox.inbox_members.includes(:user) + @inbox_members = @web_widget.inbox.inbox_members.includes(user: { avatar_attachment: :blob }) end end diff --git a/app/javascript/widget/store/modules/campaign.js b/app/javascript/widget/store/modules/campaign.js index 317c64148..ad2822dd9 100644 --- a/app/javascript/widget/store/modules/campaign.js +++ b/app/javascript/widget/store/modules/campaign.js @@ -7,6 +7,7 @@ import { const state = { records: [], uiFlags: { + hasFetched: false, isError: false, }, activeCampaign: {}, @@ -30,6 +31,7 @@ const resetCampaignTimers = ( export const getters = { getCampaigns: $state => $state.records, + getUIFlags: $state => $state.uiFlags, getActiveCampaign: $state => $state.activeCampaign, }; @@ -53,15 +55,21 @@ export const actions = { } }, initCampaigns: async ( - { getters: { getCampaigns: campaigns }, dispatch }, + { getters: { getCampaigns: campaigns, getUIFlags: uiFlags }, dispatch }, { currentURL, websiteToken, isInBusinessHours } ) => { if (!campaigns.length) { - dispatch('fetchCampaigns', { - websiteToken, - currentURL, - isInBusinessHours, - }); + // This check is added to ensure that the campaigns are fetched once + // On high traffic sites, if the campaigns are empty, the API is called + // every time the user changes the URL (in case of the SPA) + // So, we need to ensure that the campaigns are fetched only once + if (!uiFlags.hasFetched) { + dispatch('fetchCampaigns', { + websiteToken, + currentURL, + isInBusinessHours, + }); + } } else { resetCampaignTimers( campaigns, @@ -127,6 +135,7 @@ export const actions = { export const mutations = { setCampaigns($state, data) { $state.records = data; + $state.uiFlags.hasFetched = true; }, setActiveCampaign($state, data) { $state.activeCampaign = data; diff --git a/app/javascript/widget/store/modules/specs/campaign/actions.spec.js b/app/javascript/widget/store/modules/specs/campaign/actions.spec.js index ca4a9736b..c24490171 100644 --- a/app/javascript/widget/store/modules/specs/campaign/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/campaign/actions.spec.js @@ -63,15 +63,37 @@ describe('#actions', () => { }; it('sends correct actions if campaigns are empty', async () => { await actions.initCampaigns( - { dispatch, getters: { getCampaigns: [] } }, + { + dispatch, + getters: { getCampaigns: [], getUIFlags: { hasFetched: false } }, + }, actionParams ); expect(dispatch.mock.calls).toEqual([['fetchCampaigns', actionParams]]); expect(campaignTimer.initTimers).not.toHaveBeenCalled(); }); + + it('do not refetch if the campaigns are fetched once', async () => { + await actions.initCampaigns( + { + dispatch, + getters: { getCampaigns: [], getUIFlags: { hasFetched: true } }, + }, + actionParams + ); + expect(dispatch.mock.calls).toEqual([]); + expect(campaignTimer.initTimers).not.toHaveBeenCalled(); + }); + it('resets time if campaigns are available', async () => { await actions.initCampaigns( - { dispatch, getters: { getCampaigns: campaigns } }, + { + dispatch, + getters: { + getCampaigns: campaigns, + getUIFlags: { hasFetched: true }, + }, + }, actionParams ); expect(dispatch.mock.calls).toEqual([]); diff --git a/app/javascript/widget/store/modules/specs/campaign/mutations.spec.js b/app/javascript/widget/store/modules/specs/campaign/mutations.spec.js index 28efb3146..9554a9d6b 100644 --- a/app/javascript/widget/store/modules/specs/campaign/mutations.spec.js +++ b/app/javascript/widget/store/modules/specs/campaign/mutations.spec.js @@ -7,9 +7,10 @@ vi.mock('widget/store/index.js', () => ({ describe('#mutations', () => { describe('#setCampaigns', () => { it('set campaign records', () => { - const state = { records: [] }; + const state = { records: [], uiFlags: {} }; mutations.setCampaigns(state, campaigns); expect(state.records).toEqual(campaigns); + expect(state.uiFlags.hasFetched).toEqual(true); }); }); diff --git a/app/views/public/api/v1/models/_category.json.jbuilder b/app/views/public/api/v1/models/_category.json.jbuilder index 915914e57..15dc61668 100644 --- a/app/views/public/api/v1/models/_category.json.jbuilder +++ b/app/views/public/api/v1/models/_category.json.jbuilder @@ -4,26 +4,6 @@ json.locale category.locale json.description category.description json.position category.position -json.related_categories do - if category.related_categories.any? - json.array! category.related_categories.each do |related_category| - json.partial! partial: 'public/api/v1/models/associated_category', formats: [:json], category: related_category - end - end -end - -if category.parent_category.present? - json.parent_category do - json.partial! partial: 'public/api/v1/models/associated_category', formats: [:json], category: category.parent_category - end -end - -if category.root_category.present? - json.root_category do - json.partial! partial: 'public/api/v1/models/associated_category', formats: [:json], category: category.root_category - end -end - json.meta do json.articles_count category.articles.published.size end From 586dc800bb0d6772a9668eb5b4fe7c59df6fe82f Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Sat, 15 Mar 2025 13:51:08 -0700 Subject: [PATCH 02/16] chore: Move Twilio event processing to background job (#11094) - Twilio events were being processed synchronously, leading to slow API responses. - This change moves Twilio event processing to a background job to improve performance and align with how other events (e.g., WhatsApp) are handled. --------- Co-authored-by: Pranav --- app/controllers/twilio/callback_controller.rb | 2 +- .../twilio/delivery_status_controller.rb | 2 +- .../webhooks/twilio_delivery_status_job.rb | 8 ++++++ app/jobs/webhooks/twilio_events_job.rb | 8 ++++++ .../twilio/callbacks_controller_spec.rb | 28 +++++++++++++------ .../twilio/delivery_status_controller_spec.rb | 26 +++++++++++------ .../twilio_delivery_status_job_spec.rb | 26 +++++++++++++++++ spec/jobs/webhooks/twilio_events_job_spec.rb | 28 +++++++++++++++++++ 8 files changed, 108 insertions(+), 20 deletions(-) create mode 100644 app/jobs/webhooks/twilio_delivery_status_job.rb create mode 100644 app/jobs/webhooks/twilio_events_job.rb create mode 100644 spec/jobs/webhooks/twilio_delivery_status_job_spec.rb create mode 100644 spec/jobs/webhooks/twilio_events_job_spec.rb diff --git a/app/controllers/twilio/callback_controller.rb b/app/controllers/twilio/callback_controller.rb index 7723e5dd2..ff5db952d 100644 --- a/app/controllers/twilio/callback_controller.rb +++ b/app/controllers/twilio/callback_controller.rb @@ -1,6 +1,6 @@ class Twilio::CallbackController < ApplicationController def create - ::Twilio::IncomingMessageService.new(params: permitted_params).perform + Webhooks::TwilioEventsJob.perform_later(permitted_params.to_unsafe_hash) head :no_content end diff --git a/app/controllers/twilio/delivery_status_controller.rb b/app/controllers/twilio/delivery_status_controller.rb index cc7afb0fc..1c756a1c2 100644 --- a/app/controllers/twilio/delivery_status_controller.rb +++ b/app/controllers/twilio/delivery_status_controller.rb @@ -1,6 +1,6 @@ class Twilio::DeliveryStatusController < ApplicationController def create - ::Twilio::DeliveryStatusService.new(params: permitted_params).perform + Webhooks::TwilioDeliveryStatusJob.perform_later(permitted_params.to_unsafe_hash) head :no_content end diff --git a/app/jobs/webhooks/twilio_delivery_status_job.rb b/app/jobs/webhooks/twilio_delivery_status_job.rb new file mode 100644 index 000000000..324a17e94 --- /dev/null +++ b/app/jobs/webhooks/twilio_delivery_status_job.rb @@ -0,0 +1,8 @@ +class Webhooks::TwilioDeliveryStatusJob < ApplicationJob + queue_as :low + + def perform(params = {}) + # Process the Twilio delivery status webhook event in the background + ::Twilio::DeliveryStatusService.new(params: params).perform + end +end diff --git a/app/jobs/webhooks/twilio_events_job.rb b/app/jobs/webhooks/twilio_events_job.rb new file mode 100644 index 000000000..cd430a6eb --- /dev/null +++ b/app/jobs/webhooks/twilio_events_job.rb @@ -0,0 +1,8 @@ +class Webhooks::TwilioEventsJob < ApplicationJob + queue_as :low + + def perform(params = {}) + # Process the Twilio webhook event in the background + ::Twilio::IncomingMessageService.new(params: params).perform + end +end diff --git a/spec/controllers/twilio/callbacks_controller_spec.rb b/spec/controllers/twilio/callbacks_controller_spec.rb index bf975be6b..d16acf229 100644 --- a/spec/controllers/twilio/callbacks_controller_spec.rb +++ b/spec/controllers/twilio/callbacks_controller_spec.rb @@ -2,17 +2,27 @@ require 'rails_helper' RSpec.describe 'Twilio::CallbacksController', type: :request do include Rails.application.routes.url_helpers - let(:twilio_service) { instance_double(Twilio::IncomingMessageService) } - before do - allow(Twilio::IncomingMessageService).to receive(:new).and_return(twilio_service) - allow(twilio_service).to receive(:perform) - end + describe 'POST /twilio/callback' do + let(:params) do + { + 'From' => '+1234567890', + 'To' => '+0987654321', + 'Body' => 'Test message', + 'AccountSid' => 'AC123', + 'SmsSid' => 'SM123' + } + end - describe 'GET /twilio/callback' do - it 'calls incoming message service' do - post twilio_callback_index_url, params: {} - expect(twilio_service).to have_received(:perform) + it 'enqueues the Twilio events job' do + expect do + post twilio_callback_index_url, params: params + end.to have_enqueued_job(Webhooks::TwilioEventsJob).with(params) + end + + it 'returns no content status' do + post twilio_callback_index_url, params: params + expect(response).to have_http_status(:no_content) end end end diff --git a/spec/controllers/twilio/delivery_status_controller_spec.rb b/spec/controllers/twilio/delivery_status_controller_spec.rb index 05c236259..fc21f8f94 100644 --- a/spec/controllers/twilio/delivery_status_controller_spec.rb +++ b/spec/controllers/twilio/delivery_status_controller_spec.rb @@ -2,17 +2,25 @@ require 'rails_helper' RSpec.describe 'Twilio::DeliveryStatusController', type: :request do include Rails.application.routes.url_helpers - let(:twilio_service) { instance_double(Twilio::DeliveryStatusService) } - before do - allow(Twilio::DeliveryStatusService).to receive(:new).and_return(twilio_service) - allow(twilio_service).to receive(:perform) - end + describe 'POST /twilio/delivery_status' do + let(:params) do + { + 'MessageSid' => 'SM123', + 'MessageStatus' => 'delivered', + 'AccountSid' => 'AC123' + } + end - describe 'POST /twilio/delivery' do - it 'calls incoming message service' do - post twilio_delivery_status_index_url, params: {} - expect(twilio_service).to have_received(:perform) + it 'enqueues the Twilio delivery status job' do + expect do + post twilio_delivery_status_index_url, params: params + end.to have_enqueued_job(Webhooks::TwilioDeliveryStatusJob).with(params) + end + + it 'returns no content status' do + post twilio_delivery_status_index_url, params: params + expect(response).to have_http_status(:no_content) end end end diff --git a/spec/jobs/webhooks/twilio_delivery_status_job_spec.rb b/spec/jobs/webhooks/twilio_delivery_status_job_spec.rb new file mode 100644 index 000000000..dc94169ae --- /dev/null +++ b/spec/jobs/webhooks/twilio_delivery_status_job_spec.rb @@ -0,0 +1,26 @@ +require 'rails_helper' + +RSpec.describe Webhooks::TwilioDeliveryStatusJob do + subject(:job) { described_class.perform_later(params) } + + let(:params) do + { + 'MessageSid' => 'SM123', + 'MessageStatus' => 'delivered', + 'AccountSid' => 'AC123' + } + end + + it 'queues the job' do + expect { job }.to have_enqueued_job(described_class) + .with(params) + .on_queue('low') + end + + it 'calls the Twilio::DeliveryStatusService' do + service = double + expect(Twilio::DeliveryStatusService).to receive(:new).with(params: params).and_return(service) + expect(service).to receive(:perform) + described_class.new.perform(params) + end +end diff --git a/spec/jobs/webhooks/twilio_events_job_spec.rb b/spec/jobs/webhooks/twilio_events_job_spec.rb new file mode 100644 index 000000000..4eea87d6a --- /dev/null +++ b/spec/jobs/webhooks/twilio_events_job_spec.rb @@ -0,0 +1,28 @@ +require 'rails_helper' + +RSpec.describe Webhooks::TwilioEventsJob do + subject(:job) { described_class.perform_later(params) } + + let(:params) do + { + 'From' => '+1234567890', + 'To' => '+0987654321', + 'Body' => 'Test message', + 'AccountSid' => 'AC123', + 'SmsSid' => 'SM123' + } + end + + it 'queues the job' do + expect { job }.to have_enqueued_job(described_class) + .with(params) + .on_queue('low') + end + + it 'calls the Twilio::IncomingMessageService' do + service = double + expect(Twilio::IncomingMessageService).to receive(:new).with(params: params).and_return(service) + expect(service).to receive(:perform) + described_class.new.perform(params) + end +end From bf5e4a92ddd42b540a29c7f26bb6f23d29f3baf8 Mon Sep 17 00:00:00 2001 From: Pranav Date: Sat, 15 Mar 2025 14:10:12 -0700 Subject: [PATCH 03/16] chore: Limit the number of articles retrieved by widget (#11095) The UI displays only six articles, and this update introduces a per_page parameter to control the number of articles returned per API call. The value is capped between 1 and 100, with a default fallback if a lower number is set. This change is necessary due to high website traffic, where excessive payloads are returned without adding value. **Changes:** - Add index to status, account_id, portal_id, views. - Add per_page param in the API. - Update the code in the frontend to fetch only 6 --- .../api/v1/portals/articles_controller.rb | 14 +++++++++++--- app/javascript/widget/api/endPoints.js | 1 + app/models/article.rb | 4 ++++ .../api/v1/portals/articles/index.json.jbuilder | 2 +- app/views/widget_tests/index.html.erb | 3 ++- .../20250315202035_add_index_to_articles.rb | 8 ++++++++ db/schema.rb | 6 +++++- .../api/v1/portals/articles_controller_spec.rb | 17 +++++++++++++++++ 8 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20250315202035_add_index_to_articles.rb diff --git a/app/controllers/public/api/v1/portals/articles_controller.rb b/app/controllers/public/api/v1/portals/articles_controller.rb index f0fcf403d..d07dbcb9d 100644 --- a/app/controllers/public/api/v1/portals/articles_controller.rb +++ b/app/controllers/public/api/v1/portals/articles_controller.rb @@ -6,17 +6,25 @@ class Public::Api::V1::Portals::ArticlesController < Public::Api::V1::Portals::B layout 'portal' def index - @articles = @portal.articles.published + @articles = @portal.articles.published.includes(:category, :author) @articles_count = @articles.count search_articles order_by_sort_param - @articles = @articles.page(list_params[:page]) if list_params[:page].present? + limit_results end def show; end private + def limit_results + return if list_params[:per_page].blank? + + per_page = [list_params[:per_page].to_i, 100].min + per_page = 25 if per_page < 1 + @articles = @articles.page(list_params[:page]).per(per_page) + end + def search_articles @articles = @articles.search(list_params) if list_params.present? end @@ -45,7 +53,7 @@ class Public::Api::V1::Portals::ArticlesController < Public::Api::V1::Portals::B end def list_params - params.permit(:query, :locale, :sort, :status, :page) + params.permit(:query, :locale, :sort, :status, :page, :per_page) end def permitted_params diff --git a/app/javascript/widget/api/endPoints.js b/app/javascript/widget/api/endPoints.js index e6ada5914..b595fdf00 100755 --- a/app/javascript/widget/api/endPoints.js +++ b/app/javascript/widget/api/endPoints.js @@ -103,6 +103,7 @@ const getMostReadArticles = (slug, locale) => ({ page: 1, sort: 'views', status: 1, + per_page: 6, }, }); diff --git a/app/models/article.rb b/app/models/article.rb index ace793016..48e0529a2 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -23,9 +23,13 @@ # # Indexes # +# index_articles_on_account_id (account_id) # index_articles_on_associated_article_id (associated_article_id) # index_articles_on_author_id (author_id) +# index_articles_on_portal_id (portal_id) # index_articles_on_slug (slug) UNIQUE +# index_articles_on_status (status) +# index_articles_on_views (views) # class Article < ApplicationRecord include PgSearch::Model diff --git a/app/views/public/api/v1/portals/articles/index.json.jbuilder b/app/views/public/api/v1/portals/articles/index.json.jbuilder index 1927d5a09..43a9a173a 100644 --- a/app/views/public/api/v1/portals/articles/index.json.jbuilder +++ b/app/views/public/api/v1/portals/articles/index.json.jbuilder @@ -4,5 +4,5 @@ json.payload do end json.meta do - json.articles_count @articles.published.size + json.articles_count @articles_count end diff --git a/app/views/widget_tests/index.html.erb b/app/views/widget_tests/index.html.erb index cfc084384..9471c44c2 100644 --- a/app/views/widget_tests/index.html.erb +++ b/app/views/widget_tests/index.html.erb @@ -1,5 +1,6 @@ - + + <% user_id = 1 diff --git a/db/migrate/20250315202035_add_index_to_articles.rb b/db/migrate/20250315202035_add_index_to_articles.rb new file mode 100644 index 000000000..8b26344de --- /dev/null +++ b/db/migrate/20250315202035_add_index_to_articles.rb @@ -0,0 +1,8 @@ +class AddIndexToArticles < ActiveRecord::Migration[7.0] + def change + add_index :articles, :status unless index_exists?(:articles, :status) + add_index :articles, :views unless index_exists?(:articles, :views) + add_index :articles, :portal_id unless index_exists?(:articles, :portal_id) + add_index :articles, :account_id unless index_exists?(:articles, :account_id) + end +end diff --git a/db/schema.rb b/db/schema.rb index a68593c68..0818d1117 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2025_02_28_185548) do +ActiveRecord::Schema[7.0].define(version: 2025_03_15_202035) do # These extensions should be enabled to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" @@ -159,9 +159,13 @@ ActiveRecord::Schema[7.0].define(version: 2025_02_28_185548) do t.string "slug", null: false t.integer "position" t.string "locale", default: "en", null: false + t.index ["account_id"], name: "index_articles_on_account_id" t.index ["associated_article_id"], name: "index_articles_on_associated_article_id" t.index ["author_id"], name: "index_articles_on_author_id" + t.index ["portal_id"], name: "index_articles_on_portal_id" t.index ["slug"], name: "index_articles_on_slug", unique: true + t.index ["status"], name: "index_articles_on_status" + t.index ["views"], name: "index_articles_on_views" end create_table "attachments", id: :serial, force: :cascade do |t| diff --git a/spec/controllers/public/api/v1/portals/articles_controller_spec.rb b/spec/controllers/public/api/v1/portals/articles_controller_spec.rb index 249d1e78a..08c5206c1 100644 --- a/spec/controllers/public/api/v1/portals/articles_controller_spec.rb +++ b/spec/controllers/public/api/v1/portals/articles_controller_spec.rb @@ -58,6 +58,23 @@ RSpec.describe 'Public Articles API', type: :request do expect(response_data[1][:views]).to eq(1) expect(response_data.last[:id]).to eq(article.id) end + + it 'limits results based on per_page parameter' do + get "/hc/#{portal.slug}/#{category.locale}/articles.json", params: { per_page: 2 } + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true)[:payload] + expect(response_data.length).to eq(2) + expect(JSON.parse(response.body, symbolize_names: true)[:meta][:articles_count]).to eq(5) + end + + it 'uses default items per page if per_page is less than 1' do + get "/hc/#{portal.slug}/#{category.locale}/articles.json", params: { per_page: 0 } + + expect(response).to have_http_status(:success) + response_data = JSON.parse(response.body, symbolize_names: true)[:payload] + expect(response_data.length).to eq(3) + end end describe 'GET /public/api/v1/portals/:slug/articles/:id' do From 991b108a35c03b73af6366357b9e145be2ff9228 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Mon, 17 Mar 2025 00:17:11 -0700 Subject: [PATCH 04/16] feat: discard Twilio events when Body parameter is not present (#11096) - Discard Twilio events when body parameter is not present. --------- Co-authored-by: Pranav --- .../webhooks/twilio_delivery_status_job.rb | 1 - app/jobs/webhooks/twilio_events_job.rb | 5 +- spec/jobs/webhooks/twilio_events_job_spec.rb | 66 +++++++++++++++++-- 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/app/jobs/webhooks/twilio_delivery_status_job.rb b/app/jobs/webhooks/twilio_delivery_status_job.rb index 324a17e94..d57946b4c 100644 --- a/app/jobs/webhooks/twilio_delivery_status_job.rb +++ b/app/jobs/webhooks/twilio_delivery_status_job.rb @@ -2,7 +2,6 @@ class Webhooks::TwilioDeliveryStatusJob < ApplicationJob queue_as :low def perform(params = {}) - # Process the Twilio delivery status webhook event in the background ::Twilio::DeliveryStatusService.new(params: params).perform end end diff --git a/app/jobs/webhooks/twilio_events_job.rb b/app/jobs/webhooks/twilio_events_job.rb index cd430a6eb..5f44d981b 100644 --- a/app/jobs/webhooks/twilio_events_job.rb +++ b/app/jobs/webhooks/twilio_events_job.rb @@ -2,7 +2,10 @@ class Webhooks::TwilioEventsJob < ApplicationJob queue_as :low def perform(params = {}) - # Process the Twilio webhook event in the background + # Skip processing if Body parameter or MediaUrl0 is not present + # This is to skip processing delivery events being delivered to this endpoint + return if params[:Body].blank? && params[:MediaUrl0].blank? + ::Twilio::IncomingMessageService.new(params: params).perform end end diff --git a/spec/jobs/webhooks/twilio_events_job_spec.rb b/spec/jobs/webhooks/twilio_events_job_spec.rb index 4eea87d6a..f42caf675 100644 --- a/spec/jobs/webhooks/twilio_events_job_spec.rb +++ b/spec/jobs/webhooks/twilio_events_job_spec.rb @@ -5,11 +5,11 @@ RSpec.describe Webhooks::TwilioEventsJob do let(:params) do { - 'From' => '+1234567890', - 'To' => '+0987654321', - 'Body' => 'Test message', - 'AccountSid' => 'AC123', - 'SmsSid' => 'SM123' + From: '+1234567890', + To: '+0987654321', + Body: 'Test message', + AccountSid: 'AC123', + SmsSid: 'SM123' } end @@ -23,6 +23,60 @@ RSpec.describe Webhooks::TwilioEventsJob do service = double expect(Twilio::IncomingMessageService).to receive(:new).with(params: params).and_return(service) expect(service).to receive(:perform) - described_class.new.perform(params) + described_class.perform_now(params) + end + + context 'when Body parameter or MediaUrl0 is not present' do + let(:params_without_body) do + { + From: '+1234567890', + To: '+0987654321', + AccountSid: 'AC123', + SmsSid: 'SM123' + } + end + + it 'does not process the event' do + expect(Twilio::IncomingMessageService).not_to receive(:new) + described_class.perform_now(params_without_body) + end + end + + context 'when Body parameter is present' do + let(:params_with_body) do + { + From: '+1234567890', + To: '+0987654321', + Body: 'Test message', + AccountSid: 'AC123', + SmsSid: 'SM123' + } + end + + it 'processes the event' do + service = double + expect(Twilio::IncomingMessageService).to receive(:new).with(params: params_with_body).and_return(service) + expect(service).to receive(:perform) + described_class.perform_now(params_with_body) + end + end + + context 'when MediaUrl0 parameter is present' do + let(:params_with_media) do + { + From: '+1234567890', + To: '+0987654321', + MediaUrl0: 'https://example.com/media.jpg', + AccountSid: 'AC123', + SmsSid: 'SM123' + } + end + + it 'processes the event' do + service = double + expect(Twilio::IncomingMessageService).to receive(:new).with(params: params_with_media).and_return(service) + expect(service).to receive(:perform) + described_class.perform_now(params_with_media) + end end end From 1c42957891d0a5e5249127aaa7c0bfa156083812 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Tue, 18 Mar 2025 07:57:05 +0530 Subject: [PATCH 05/16] perf: enable active record connection pool reaper (#10866) The pg_stat_activity data showed a lot of idle connections. This affects the total number of database connections available which resulted in production incidents during the weekend. > reaping_frequency: frequency in seconds to periodically run the Reaper, which attempts to find and recover connections from dead threads, which can occur if a programmer forgets to close a connection at the end of a thread or a thread dies unexpectedly. Regardless of this setting, the Reaper will be invoked before every blocking wait. (Default nil, which means don't schedule the Reaper). Ref: https://api.rubyonrails.org/v5.1/classes/ActiveRecord/ConnectionAdapters/ConnectionPool.html --- config/database.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/database.yml b/config/database.yml index 5cd278f8d..0577e3ee6 100644 --- a/config/database.yml +++ b/config/database.yml @@ -5,6 +5,9 @@ default: &default port: <%= ENV.fetch('POSTGRES_PORT', '5432') %> # ref: https://github.com/mperham/sidekiq/issues/2985#issuecomment-531097962 pool: <%= Sidekiq.server? ? ENV.fetch('SIDEKIQ_CONCURRENCY', 10) : ENV.fetch('RAILS_MAX_THREADS', 5) %> + # frequency in seconds to periodically run the Reaper, which attempts + # to find and recover connections from dead threads + reaping_frequency: <%= ENV.fetch('DB_POOL_REAPING_FREQUENCY', 30) %> variables: # we are setting this value to be close to the racktimeout value. we will iterate and reduce this value going forward statement_timeout: <%= ENV["POSTGRES_STATEMENT_TIMEOUT"] || "14s" %> From 3dc7045340ecb81cfcd2cda2f87e87e33d362787 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Tue, 18 Mar 2025 09:21:56 +0530 Subject: [PATCH 06/16] fix: Dropdown for custom attributes in conversation sidebar hides under the list (#11099) # Pull Request Template ## Description This PR fixes the issue where the custom attributes type list dropdown in the conversation sidebar gets hidden under the section. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ### Screenshots **Before** image **After** image ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- .../conversation/customAttributes/CustomAttributes.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/dashboard/routes/dashboard/conversation/customAttributes/CustomAttributes.vue b/app/javascript/dashboard/routes/dashboard/conversation/customAttributes/CustomAttributes.vue index 01fbe9b71..4f0cf6340 100644 --- a/app/javascript/dashboard/routes/dashboard/conversation/customAttributes/CustomAttributes.vue +++ b/app/javascript/dashboard/routes/dashboard/conversation/customAttributes/CustomAttributes.vue @@ -271,7 +271,7 @@ const evenClass = [ ghost-class="ghost" handle=".drag-handle" item-key="key" - class="last:rounded-b-lg overflow-hidden" + class="last:rounded-b-lg" :class="evenClass" @start="dragging = true" @end="onDragEnd" From bbfcdb3d428b7485e42aeedf9435b3e8c642a7f5 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Tue, 18 Mar 2025 14:01:18 +0530 Subject: [PATCH 07/16] chore: Improvements in image attachment viewer (#11040) This PR includes improvements in image attachment/gallery viewer: 1. Added double-click zoom functionality (depreciated click to zoom) 2. Implemented scroll zoom based on cursor position 3. Increase the zoom scale 4. Improved layout and styling for better usability Fixes https://linear.app/chatwoot/issue/CW-4127/zoom-images-from-a-specific-location ## How Has This Been Tested? Loom video https://www.loom.com/share/b21e00db3bc74231a90202eb6eb2fb5a?sid=a0651bf1-0952-430b-a5a9-83bf0858e059 --------- Co-authored-by: Pranav Co-authored-by: Shivam Mishra --- .../conversation/components/GalleryView.vue | 255 ++++++++---------- .../composables/spec/useImageZoom.spec.js | 141 ++++++++++ .../dashboard/composables/useImageZoom.js | 186 +++++++++++++ package.json | 2 +- pnpm-lock.yaml | 10 +- 5 files changed, 439 insertions(+), 155 deletions(-) create mode 100644 app/javascript/dashboard/composables/spec/useImageZoom.spec.js create mode 100644 app/javascript/dashboard/composables/useImageZoom.js diff --git a/app/javascript/dashboard/components/widgets/conversation/components/GalleryView.vue b/app/javascript/dashboard/components/widgets/conversation/components/GalleryView.vue index f6e496ce6..5c56639f4 100644 --- a/app/javascript/dashboard/components/widgets/conversation/components/GalleryView.vue +++ b/app/javascript/dashboard/components/widgets/conversation/components/GalleryView.vue @@ -1,10 +1,11 @@ - - - - diff --git a/app/javascript/dashboard/routes/dashboard/conversation/contact/ContactInfo.vue b/app/javascript/dashboard/routes/dashboard/conversation/contact/ContactInfo.vue index 0de55a92e..b423ecfc6 100644 --- a/app/javascript/dashboard/routes/dashboard/conversation/contact/ContactInfo.vue +++ b/app/javascript/dashboard/routes/dashboard/conversation/contact/ContactInfo.vue @@ -7,8 +7,8 @@ import ContactInfoRow from './ContactInfoRow.vue'; import Thumbnail from 'dashboard/components/widgets/Thumbnail.vue'; import SocialIcons from './SocialIcons.vue'; import EditContact from './EditContact.vue'; -import NewConversation from './NewConversation.vue'; import ContactMergeModal from 'dashboard/modules/contact/ContactMergeModal.vue'; +import ComposeConversation from 'dashboard/components-next/NewConversation/ComposeConversation.vue'; import { BUS_EVENTS } from 'shared/constants/busEvents'; import NextButton from 'dashboard/components-next/button/Button.vue'; @@ -25,8 +25,8 @@ export default { ContactInfoRow, EditContact, Thumbnail, + ComposeConversation, SocialIcons, - NewConversation, ContactMergeModal, }, props: { @@ -49,7 +49,6 @@ export default { data() { return { showEditModal: false, - showConversationModal: false, showMergeModal: false, showDeleteModal: false, }; @@ -92,17 +91,29 @@ export default { return ` ${this.contact.name}?`; }, }, + watch: { + 'contact.id': { + handler(id) { + this.$store.dispatch('contacts/fetchContactableInbox', id); + }, + immediate: true, + }, + }, methods: { dynamicTime, toggleEditModal() { this.showEditModal = !this.showEditModal; }, - toggleConversationModal() { - this.showConversationModal = !this.showConversationModal; - emitter.emit( - BUS_EVENTS.NEW_CONVERSATION_MODAL, - this.showConversationModal - ); + openComposeConversationModal(toggleFn) { + toggleFn(); + // Flag to prevent triggering drag n drop, + // When compose modal is active + emitter.emit(BUS_EVENTS.NEW_CONVERSATION_MODAL, true); + }, + closeComposeConversationModal() { + // Flag to enable drag n drop, + // When compose modal is closed + emitter.emit(BUS_EVENTS.NEW_CONVERSATION_MODAL, false); }, toggleDeleteModal() { this.showDeleteModal = !this.showDeleteModal; @@ -113,7 +124,6 @@ export default { }, closeDelete() { this.showDeleteModal = false; - this.showConversationModal = false; this.showEditModal = false; }, findCountryFlag(countryCode, cityAndCountry) { @@ -250,14 +260,22 @@ export default {
- + + + - -import { ref } from 'vue'; -// constants & helpers -import { ALLOWED_FILE_TYPES } from 'shared/constants/messages'; -import { ExceptionWithMessage } from 'shared/helpers/CustomErrors'; -import { getInboxSource, INBOX_TYPES } from 'dashboard/helper/inbox'; - -// store -import { mapGetters } from 'vuex'; - -// composables -import { useUISettings } from 'dashboard/composables/useUISettings'; -import { useAlert } from 'dashboard/composables'; -import { required, requiredIf } from '@vuelidate/validators'; -import { useVuelidate } from '@vuelidate/core'; - -// mixins -import fileUploadMixin from 'dashboard/mixins/fileUploadMixin'; -import inboxMixin from 'shared/mixins/inboxMixin'; - -// components -import AttachmentPreview from 'dashboard/components/widgets/AttachmentsPreview.vue'; -import CannedResponse from 'dashboard/components/widgets/conversation/CannedResponse.vue'; -import InboxDropdownItem from 'dashboard/components/widgets/InboxDropdownItem.vue'; -import MessageSignatureMissingAlert from 'dashboard/components/widgets/conversation/MessageSignatureMissingAlert.vue'; -import ReplyEmailHead from 'dashboard/components/widgets/conversation/ReplyEmailHead.vue'; -import WootMessageEditor from 'dashboard/components/widgets/WootWriter/Editor.vue'; -import Thumbnail from 'dashboard/components/widgets/Thumbnail.vue'; -import FileUpload from 'vue-upload-component'; -import WhatsappTemplates from './WhatsappTemplates.vue'; - -import { - appendSignature, - removeSignature, -} from 'dashboard/helper/editorHelper'; - -export default { - components: { - Thumbnail, - WootMessageEditor, - ReplyEmailHead, - CannedResponse, - WhatsappTemplates, - InboxDropdownItem, - FileUpload, - AttachmentPreview, - MessageSignatureMissingAlert, - }, - mixins: [inboxMixin, fileUploadMixin], - props: { - contact: { - type: Object, - default: () => ({}), - }, - onSubmit: { - type: Function, - default: () => {}, - }, - }, - emits: ['cancel', 'success'], - setup() { - const { fetchSignatureFlagFromUISettings, setSignatureFlagForInbox } = - useUISettings(); - const v$ = useVuelidate(); - const uploadAttachment = ref(false); - - return { - fetchSignatureFlagFromUISettings, - setSignatureFlagForInbox, - v$, - uploadAttachment, - }; - }, - data() { - return { - name: '', - subject: '', - message: '', - showCannedResponseMenu: false, - cannedResponseSearchKey: '', - bccEmails: '', - ccEmails: '', - targetInbox: {}, - whatsappTemplateSelected: false, - attachedFiles: [], - }; - }, - validations() { - return { - subject: { - required: requiredIf(this.isAnEmailInbox), - }, - message: { - required, - }, - targetInbox: { - required, - }, - }; - }, - computed: { - ...mapGetters({ - uiFlags: 'contacts/getUIFlags', - conversationsUiFlags: 'contactConversations/getUIFlags', - currentUser: 'getCurrentUser', - globalConfig: 'globalConfig/get', - messageSignature: 'getMessageSignature', - inboxesList: 'inboxes/getInboxes', - }), - sendWithSignature() { - return this.fetchSignatureFlagFromUISettings(this.channelType); - }, - signatureToApply() { - return this.messageSignature; - }, - newMessagePayload() { - const payload = { - inboxId: this.targetInbox.id, - sourceId: this.targetInbox.sourceId, - contactId: this.contact.id, - message: { content: this.message }, - mailSubject: this.subject, - assigneeId: this.currentUser.id, - }; - - if (this.attachedFiles && this.attachedFiles.length) { - payload.files = []; - this.setAttachmentPayload(payload); - } - - if (this.ccEmails) { - payload.message.cc_emails = this.ccEmails; - } - - if (this.bccEmails) { - payload.message.bcc_emails = this.bccEmails; - } - return payload; - }, - selectedInbox: { - get() { - const inboxList = this.contact.contact_inboxes || []; - const selectedContactInbox = inboxList.find( - inbox => inbox.inbox?.id && inbox.inbox?.id === this.targetInbox?.id - ); - - if (!selectedContactInbox) { - return { inbox: {} }; - } - - // Find the matching inbox from the inboxesList - const matchingInbox = - this.inboxesList.find( - item => item.id === selectedContactInbox.inbox?.id - ) || {}; - - // The entire inbox payload is not available in this object, so we need to patch it from the store - return { - ...selectedContactInbox, - inbox: { - ...matchingInbox, - ...selectedContactInbox.inbox, - sourceId: selectedContactInbox.source_id || matchingInbox.sourceId, - }, - }; - }, - set(value) { - this.targetInbox = value.inbox; - }, - }, - showNoInboxAlert() { - if (!this.contact.contact_inboxes) { - return false; - } - return this.inboxes.length === 0 && !this.uiFlags.isFetchingInboxes; - }, - isSignatureEnabledForInbox() { - return this.isAnEmailInbox && this.sendWithSignature; - }, - signatureToggleTooltip() { - return this.sendWithSignature - ? this.$t('CONVERSATION.FOOTER.DISABLE_SIGN_TOOLTIP') - : this.$t('CONVERSATION.FOOTER.ENABLE_SIGN_TOOLTIP'); - }, - - inboxes() { - const inboxList = this.contact.contact_inboxes || []; - if (!inboxList.length) return []; - - return inboxList.map(inbox => { - const matchingInbox = - this.inboxesList.find(item => item.id === inbox.inbox?.id) || {}; - - // Create merged object with a clear property order - return { - ...matchingInbox, - ...inbox.inbox, - sourceId: inbox.source_id, - }; - }); - }, - isAnEmailInbox() { - return ( - this.selectedInbox && - this.selectedInbox.inbox.channel_type === INBOX_TYPES.EMAIL - ); - }, - isAnWebWidgetInbox() { - return ( - this.selectedInbox && - this.selectedInbox.inbox.channel_type === INBOX_TYPES.WEB - ); - }, - isEmailOrWebWidgetInbox() { - return this.isAnEmailInbox || this.isAnWebWidgetInbox; - }, - hasWhatsappTemplates() { - return !!this.selectedInbox.inbox?.message_templates; - }, - hasAttachments() { - return this.attachedFiles.length; - }, - inbox() { - return this.targetInbox; - }, - allowedFileTypes() { - return ALLOWED_FILE_TYPES; - }, - }, - watch: { - message(value) { - this.hasSlashCommand = value[0] === '/' && !this.isEmailOrWebWidgetInbox; - const hasNextWord = value.includes(' '); - const isShortCodeActive = this.hasSlashCommand && !hasNextWord; - if (isShortCodeActive) { - this.cannedResponseSearchKey = value.substring(1); - this.showCannedResponseMenu = true; - } else { - this.cannedResponseSearchKey = ''; - this.showCannedResponseMenu = false; - } - }, - targetInbox() { - this.setSignature(); - }, - }, - mounted() { - this.setSignature(); - }, - methods: { - setSignature() { - if (this.messageSignature) { - if (this.isSignatureEnabledForInbox) { - this.message = appendSignature(this.message, this.signatureToApply); - } else { - this.message = removeSignature(this.message, this.signatureToApply); - } - } - }, - setAttachmentPayload(payload) { - this.attachedFiles.forEach(attachment => { - if (this.globalConfig.directUploadsEnabled) { - payload.files.push(attachment.blobSignedId); - } else { - payload.files.push(attachment.resource.file); - } - }); - }, - attachFile({ blob, file }) { - const reader = new FileReader(); - reader.readAsDataURL(file.file); - reader.onloadend = () => { - this.attachedFiles.push({ - currentChatId: this.contact.id, - resource: blob || file, - isPrivate: this.isPrivate, - thumb: reader.result, - blobSignedId: blob ? blob.signed_id : undefined, - }); - }; - }, - removeAttachment(attachments) { - this.attachedFiles = attachments; - }, - onCancel() { - this.$emit('cancel'); - }, - onSuccess() { - this.$emit('success'); - }, - replaceTextWithCannedResponse(message) { - this.message = message; - }, - toggleCannedMenu(value) { - this.showCannedMenu = value; - }, - prepareWhatsAppMessagePayload({ message: content, templateParams }) { - const payload = { - inboxId: this.targetInbox.id, - sourceId: this.targetInbox.sourceId, - contactId: this.contact.id, - message: { content, template_params: templateParams }, - assigneeId: this.currentUser.id, - }; - return payload; - }, - onFormSubmit() { - const isFromWhatsApp = false; - this.v$.$touch(); - if (this.v$.$invalid) { - return; - } - this.createConversation({ - payload: this.newMessagePayload, - isFromWhatsApp, - }); - }, - async createConversation({ payload, isFromWhatsApp }) { - try { - const data = await this.onSubmit(payload, isFromWhatsApp); - const action = { - type: 'link', - to: `/app/accounts/${data.account_id}/conversations/${data.id}`, - message: this.$t('NEW_CONVERSATION.FORM.GO_TO_CONVERSATION'), - }; - this.onSuccess(); - useAlert(this.$t('NEW_CONVERSATION.FORM.SUCCESS_MESSAGE'), action); - } catch (error) { - if (error instanceof ExceptionWithMessage) { - useAlert(error.data); - } else { - useAlert(this.$t('NEW_CONVERSATION.FORM.ERROR_MESSAGE')); - } - } - }, - - toggleWaTemplate(val) { - this.whatsappTemplateSelected = val; - }, - async onSendWhatsAppReply(messagePayload) { - const isFromWhatsApp = true; - const payload = this.prepareWhatsAppMessagePayload(messagePayload); - await this.createConversation({ payload, isFromWhatsApp }); - }, - inboxReadableIdentifier(inbox) { - return `${inbox.name} (${inbox.channel_type})`; - }, - computedInboxSource(inbox) { - if (!inbox.channel_type) return ''; - const classByType = getInboxSource( - inbox.channel_type, - inbox.phone_number, - inbox - ); - return classByType; - }, - toggleMessageSignature() { - this.setSignatureFlagForInbox(this.channelType, !this.sendWithSignature); - this.setSignature(); - }, - }, -}; - - - -