From 20a0d381a7a116533dd1dcc5752ca2eb3932b293 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Tue, 18 May 2021 12:15:23 +0530 Subject: [PATCH] fix: Resolve infinite loop with campaign API call (#2290) Co-authored-by: Muhsin --- app/javascript/widget/App.vue | 2 +- .../widget/helpers/campaignHelper.js | 8 ++-- .../widget/helpers/campaignTimer.js | 4 +- .../helpers/specs/campaignHelper.spec.js | 4 +- .../widget/store/modules/campaign.js | 37 +++++++++---------- .../modules/specs/campaign/actions.spec.js | 37 ++++++++++++++++++- .../modules/specs/campaign/getters.spec.js | 4 +- .../modules/specs/campaign/mutations.spec.js | 2 +- 8 files changed, 65 insertions(+), 33 deletions(-) diff --git a/app/javascript/widget/App.vue b/app/javascript/widget/App.vue index 6686c97fe..df1341f2c 100755 --- a/app/javascript/widget/App.vue +++ b/app/javascript/widget/App.vue @@ -36,7 +36,7 @@ export default { ...mapGetters({ hasFetched: 'agent/getHasFetched', unreadMessageCount: 'conversation/getUnreadMessageCount', - campaigns: 'campaign/fetchCampaigns', + campaigns: 'campaign/getCampaigns', }), isLeftAligned() { const isLeft = this.widgetPosition === 'left'; diff --git a/app/javascript/widget/helpers/campaignHelper.js b/app/javascript/widget/helpers/campaignHelper.js index e3bb261a3..4cde814f6 100644 --- a/app/javascript/widget/helpers/campaignHelper.js +++ b/app/javascript/widget/helpers/campaignHelper.js @@ -3,8 +3,8 @@ export const stripTrailingSlash = ({ URL }) => { }; // Format all campaigns -export const formatCampaigns = ({ campagins }) => { - return campagins.map(item => { +export const formatCampaigns = ({ campaigns }) => { + return campaigns.map(item => { return { id: item.id, timeOnPage: item?.trigger_rules?.time_on_page, @@ -14,8 +14,8 @@ export const formatCampaigns = ({ campagins }) => { }; // Find all campaigns that matches the current URL -export const filterCampaigns = ({ campagins, currentURL }) => { - return campagins.filter( +export const filterCampaigns = ({ campaigns, currentURL }) => { + return campaigns.filter( item => stripTrailingSlash({ URL: item.url }) === stripTrailingSlash({ URL: currentURL }) diff --git a/app/javascript/widget/helpers/campaignTimer.js b/app/javascript/widget/helpers/campaignTimer.js index 72a042d28..11197f1d1 100644 --- a/app/javascript/widget/helpers/campaignTimer.js +++ b/app/javascript/widget/helpers/campaignTimer.js @@ -5,9 +5,9 @@ class CampaignTimer { this.campaignTimers = []; } - initTimers = ({ campagins }) => { + initTimers = ({ campaigns }) => { this.clearTimers(); - campagins.forEach(campaign => { + campaigns.forEach(campaign => { const { timeOnPage, id: campaignId } = campaign; this.campaignTimers[campaignId] = setTimeout(() => { triggerCampaign({ campaignId }); diff --git a/app/javascript/widget/helpers/specs/campaignHelper.spec.js b/app/javascript/widget/helpers/specs/campaignHelper.spec.js index 359126ed2..db6da98f4 100644 --- a/app/javascript/widget/helpers/specs/campaignHelper.spec.js +++ b/app/javascript/widget/helpers/specs/campaignHelper.spec.js @@ -15,7 +15,7 @@ describe('#Campagin Helper', () => { describe('formatCampaigns', () => { it('should return formated campaigns if camapgins are passed', () => { - expect(formatCampaigns({ campagins: campaigns })).toStrictEqual([ + expect(formatCampaigns({ campaigns })).toStrictEqual([ { id: 1, timeOnPage: 3, @@ -33,7 +33,7 @@ describe('#Campagin Helper', () => { it('should return filtered campaigns if formatted camapgins are passed', () => { expect( filterCampaigns({ - campagins: [ + campaigns: [ { id: 1, timeOnPage: 3, diff --git a/app/javascript/widget/store/modules/campaign.js b/app/javascript/widget/store/modules/campaign.js index 74cdb1ea7..ebfd90fac 100644 --- a/app/javascript/widget/store/modules/campaign.js +++ b/app/javascript/widget/store/modules/campaign.js @@ -13,43 +13,42 @@ const state = { }, }; +const resetCampaignTimers = (campaigns, currentURL) => { + const formattedCampaigns = formatCampaigns({ campaigns }); + // Find all campaigns that matches the current URL + const filteredCampaigns = filterCampaigns({ + campaigns: formattedCampaigns, + currentURL, + }); + campaignTimer.initTimers({ campaigns: filteredCampaigns }); +}; + export const getters = { getHasFetched: $state => $state.uiFlags.hasFetched, - fetchCampaigns: $state => $state.records, + getCampaigns: $state => $state.records, }; export const actions = { - fetchCampaigns: async ( - { commit, dispatch }, - { websiteToken, currentURL } - ) => { + fetchCampaigns: async ({ commit }, { websiteToken, currentURL }) => { try { - const { data } = await getCampaigns(websiteToken); - commit('setCampaigns', data); - dispatch('startCampaigns', { currentURL, websiteToken }); + const { data: campaigns } = await getCampaigns(websiteToken); + commit('setCampaigns', campaigns); commit('setError', false); commit('setHasFetched', true); + resetCampaignTimers(campaigns, currentURL); } catch (error) { commit('setError', true); commit('setHasFetched', true); } }, startCampaigns: async ( - { getters: { fetchCampaigns: campagins }, dispatch }, + { getters: { getCampaigns: campaigns }, dispatch }, { currentURL, websiteToken } ) => { - if (!campagins.length) { + if (!campaigns.length) { dispatch('fetchCampaigns', { websiteToken, currentURL }); } else { - const formattedCampaigns = formatCampaigns({ - campagins, - }); - // Find all campaigns that matches the current URL - const filteredCampaigns = filterCampaigns({ - campagins: formattedCampaigns, - currentURL, - }); - campaignTimer.initTimers({ campagins: filteredCampaigns }); + resetCampaignTimers(campaigns, currentURL); } }, }; 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 ccfa03bf8..2a01dbf83 100644 --- a/app/javascript/widget/store/modules/specs/campaign/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/campaign/actions.spec.js @@ -3,21 +3,28 @@ import { actions } from '../../campaign'; import { campaigns } from './data'; const commit = jest.fn(); +const dispatch = jest.fn(); jest.mock('widget/helpers/axios'); +import campaignTimer from 'widget/helpers/campaignTimer'; +jest.mock('widget/helpers/campaignTimer'); + describe('#actions', () => { describe('#fetchCampaigns', () => { it('sends correct actions if API is success', async () => { API.get.mockResolvedValue({ data: campaigns }); await actions.fetchCampaigns( { commit }, - { websiteToken: 'XDsafmADasd', currentURL: 'https://www.chatwoot.com' } + { websiteToken: 'XDsafmADasd', currentURL: 'https://chatwoot.com' } ); - expect(commit.mock.calls).not.toEqual([ + expect(commit.mock.calls).toEqual([ ['setCampaigns', campaigns], ['setError', false], ['setHasFetched', true], ]); + expect(campaignTimer.initTimers).toHaveBeenCalledWith({ + campaigns: [{ id: 11, timeOnPage: '20', url: 'https://chatwoot.com' }], + }); }); it('sends correct actions if API is error', async () => { API.get.mockRejectedValue({ message: 'Authentication required' }); @@ -31,4 +38,30 @@ describe('#actions', () => { ]); }); }); + + describe('#startCampaigns', () => { + const actionParams = { + websiteToken: 'XDsafmADasd', + currentURL: 'https://chatwoot.com', + }; + + it('sends correct actions if campaigns are empty', async () => { + await actions.startCampaigns( + { dispatch, getters: { getCampaigns: [] } }, + actionParams + ); + expect(dispatch.mock.calls).toEqual([['fetchCampaigns', actionParams]]); + expect(campaignTimer.initTimers).not.toHaveBeenCalled(); + }); + it('resets time if campaigns are available', async () => { + await actions.startCampaigns( + { dispatch, getters: { getCampaigns: campaigns } }, + actionParams + ); + expect(dispatch.mock.calls).toEqual([]); + expect(campaignTimer.initTimers).toHaveBeenCalledWith({ + campaigns: [{ id: 11, timeOnPage: '20', url: 'https://chatwoot.com' }], + }); + }); + }); }); diff --git a/app/javascript/widget/store/modules/specs/campaign/getters.spec.js b/app/javascript/widget/store/modules/specs/campaign/getters.spec.js index b0a1d29aa..60af8cced 100644 --- a/app/javascript/widget/store/modules/specs/campaign/getters.spec.js +++ b/app/javascript/widget/store/modules/specs/campaign/getters.spec.js @@ -2,11 +2,11 @@ import { getters } from '../../campaign'; import { campaigns } from './data'; describe('#getters', () => { - it('fetchCampaigns', () => { + it('getCampaigns', () => { const state = { records: campaigns, }; - expect(getters.fetchCampaigns(state)).toEqual([ + expect(getters.getCampaigns(state)).toEqual([ { id: 1, title: 'Welcome', 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 63e4b4ae3..fc8c585c9 100644 --- a/app/javascript/widget/store/modules/specs/campaign/mutations.spec.js +++ b/app/javascript/widget/store/modules/specs/campaign/mutations.spec.js @@ -2,7 +2,7 @@ import { mutations } from '../../campaign'; import { campaigns } from './data'; describe('#mutations', () => { - describe('#setCampagins', () => { + describe('#setCampaigns', () => { it('set campaign records', () => { const state = { records: [] }; mutations.setCampaigns(state, campaigns);