From 46ec92c86ea8f8d36272e5b31e2f5a1bb04d1a65 Mon Sep 17 00:00:00 2001 From: Pranav Date: Fri, 14 Mar 2025 17:37:36 -0700 Subject: [PATCH] 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