diff --git a/app/builders/contact_inbox_builder.rb b/app/builders/contact_inbox_builder.rb index 4d51700a8..ffa45db2e 100644 --- a/app/builders/contact_inbox_builder.rb +++ b/app/builders/contact_inbox_builder.rb @@ -12,11 +12,50 @@ class ContactInboxBuilder private def generate_source_id - ContactInbox::SourceIdService.new( - contact: @contact, - channel_type: @inbox.channel_type, - medium: @inbox.channel.try(:medium) - ).generate + case @inbox.channel_type + when 'Channel::TwilioSms' + twilio_source_id + when 'Channel::Whatsapp' + wa_source_id + when 'Channel::Email' + email_source_id + when 'Channel::Sms' + phone_source_id + when 'Channel::Api', 'Channel::WebWidget' + SecureRandom.uuid + else + raise "Unsupported operation for this channel: #{@inbox.channel_type}" + end + end + + def email_source_id + raise ActionController::ParameterMissing, 'contact email' unless @contact.email + + @contact.email + end + + def phone_source_id + raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number + + @contact.phone_number + end + + def wa_source_id + raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number + + # whatsapp doesn't want the + in e164 format + @contact.phone_number.delete('+').to_s + end + + def twilio_source_id + raise ActionController::ParameterMissing, 'contact phone number' unless @contact.phone_number + + case @inbox.channel.medium + when 'sms' + @contact.phone_number + when 'whatsapp' + "whatsapp:#{@contact.phone_number}" + end end def create_contact_inbox @@ -52,7 +91,7 @@ class ContactInboxBuilder def new_source_id if @inbox.whatsapp? || @inbox.sms? || @inbox.twilio? - "#{@source_id}#{rand(100)}" + "whatsapp:#{@source_id}#{rand(100)}" else "#{rand(10)}#{@source_id}" end diff --git a/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb b/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb index bde9a4f0c..d985c8a73 100644 --- a/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/contacts/contact_inboxes_controller.rb @@ -9,8 +9,6 @@ class Api::V1::Accounts::Contacts::ContactInboxesController < Api::V1::Accounts: source_id: params[:source_id], hmac_verified: hmac_verified? ).perform - rescue ArgumentError => e - render json: { error: e.message }, status: :unprocessable_entity end private diff --git a/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js b/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js index 45abef1e0..57e819245 100644 --- a/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js +++ b/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js @@ -25,11 +25,6 @@ export const generateLabelForContactableInboxesList = ({ channelType === INBOX_TYPES.TWILIO || channelType === INBOX_TYPES.WHATSAPP ) { - // Handled separately for Twilio Inbox where phone number is not mandatory. - // You can send message to a contact with Messaging Service Id. - if (!phoneNumber) { - return name; - } return `${name} (${phoneNumber})`; } return name; diff --git a/app/javascript/dashboard/components-next/NewConversation/helpers/specs/composeConversationHelper.spec.js b/app/javascript/dashboard/components-next/NewConversation/helpers/specs/composeConversationHelper.spec.js index 37ae5fe29..3de1fad0d 100644 --- a/app/javascript/dashboard/components-next/NewConversation/helpers/specs/composeConversationHelper.spec.js +++ b/app/javascript/dashboard/components-next/NewConversation/helpers/specs/composeConversationHelper.spec.js @@ -8,8 +8,8 @@ vi.mock('dashboard/api/contacts'); describe('composeConversationHelper', () => { describe('generateLabelForContactableInboxesList', () => { const contact = { - name: 'Priority Inbox', - email: 'hello@example.com', + name: 'John Doe', + email: 'john@example.com', phoneNumber: '+1234567890', }; @@ -19,7 +19,7 @@ describe('composeConversationHelper', () => { ...contact, channelType: INBOX_TYPES.EMAIL, }) - ).toBe('Priority Inbox (hello@example.com)'); + ).toBe('John Doe (john@example.com)'); }); it('generates label for twilio inbox', () => { @@ -28,14 +28,7 @@ describe('composeConversationHelper', () => { ...contact, channelType: INBOX_TYPES.TWILIO, }) - ).toBe('Priority Inbox (+1234567890)'); - - expect( - helpers.generateLabelForContactableInboxesList({ - name: 'Priority Inbox', - channelType: INBOX_TYPES.TWILIO, - }) - ).toBe('Priority Inbox'); + ).toBe('John Doe (+1234567890)'); }); it('generates label for whatsapp inbox', () => { @@ -44,7 +37,7 @@ describe('composeConversationHelper', () => { ...contact, channelType: INBOX_TYPES.WHATSAPP, }) - ).toBe('Priority Inbox (+1234567890)'); + ).toBe('John Doe (+1234567890)'); }); it('generates label for other inbox types', () => { @@ -53,7 +46,7 @@ describe('composeConversationHelper', () => { ...contact, channelType: 'Channel::Api', }) - ).toBe('Priority Inbox'); + ).toBe('John Doe'); }); }); diff --git a/app/javascript/shared/helpers/cache.js b/app/javascript/shared/helpers/cache.js new file mode 100644 index 000000000..5053c4cff --- /dev/null +++ b/app/javascript/shared/helpers/cache.js @@ -0,0 +1,43 @@ +import { LocalStorage } from './localStorage'; + +// Default cache expiry is 24 hours +const DEFAULT_EXPIRY = 24 * 60 * 60 * 1000; + +export const getFromCache = (key, expiry = DEFAULT_EXPIRY) => { + try { + const cached = LocalStorage.get(key); + if (!cached) return null; + + const { data, timestamp } = cached; + const isExpired = Date.now() - timestamp > expiry; + + if (isExpired) { + LocalStorage.remove(key); + return null; + } + + return data; + } catch (error) { + return null; + } +}; + +export const setCache = (key, data) => { + try { + const cacheData = { + data, + timestamp: Date.now(), + }; + LocalStorage.set(key, cacheData); + } catch (error) { + // Ignore cache errors + } +}; + +export const clearCache = key => { + try { + LocalStorage.remove(key); + } catch (error) { + // Ignore cache errors + } +}; diff --git a/app/javascript/shared/helpers/specs/cache.spec.js b/app/javascript/shared/helpers/specs/cache.spec.js new file mode 100644 index 000000000..0a44a5605 --- /dev/null +++ b/app/javascript/shared/helpers/specs/cache.spec.js @@ -0,0 +1,136 @@ +import { getFromCache, setCache, clearCache } from '../cache'; +import { LocalStorage } from '../localStorage'; + +vi.mock('../localStorage'); + +describe('Cache Helpers', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + vi.setSystemTime(new Date(2023, 1, 1, 0, 0, 0)); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('getFromCache', () => { + it('returns null when no data is cached', () => { + LocalStorage.get.mockReturnValue(null); + + const result = getFromCache('test-key'); + + expect(result).toBeNull(); + expect(LocalStorage.get).toHaveBeenCalledWith('test-key'); + }); + + it('returns cached data when not expired', () => { + // Current time is 2023-02-01 00:00:00 + // Cache timestamp is 1 hour ago + const oneHourAgo = + new Date(2023, 1, 1, 0, 0, 0).getTime() - 60 * 60 * 1000; + + LocalStorage.get.mockReturnValue({ + data: { foo: 'bar' }, + timestamp: oneHourAgo, + }); + + // Default expiry is 24 hours + const result = getFromCache('test-key'); + + expect(result).toEqual({ foo: 'bar' }); + expect(LocalStorage.get).toHaveBeenCalledWith('test-key'); + expect(LocalStorage.remove).not.toHaveBeenCalled(); + }); + + it('removes and returns null when data is expired', () => { + // Current time is 2023-02-01 00:00:00 + // Cache timestamp is 25 hours ago (beyond the default 24-hour expiry) + const twentyFiveHoursAgo = + new Date(2023, 1, 1, 0, 0, 0).getTime() - 25 * 60 * 60 * 1000; + + LocalStorage.get.mockReturnValue({ + data: { foo: 'bar' }, + timestamp: twentyFiveHoursAgo, + }); + + const result = getFromCache('test-key'); + + expect(result).toBeNull(); + expect(LocalStorage.get).toHaveBeenCalledWith('test-key'); + expect(LocalStorage.remove).toHaveBeenCalledWith('test-key'); + }); + + it('respects custom expiry time', () => { + // Current time is 2023-02-01 00:00:00 + // Cache timestamp is 2 hours ago + const twoHoursAgo = + new Date(2023, 1, 1, 0, 0, 0).getTime() - 2 * 60 * 60 * 1000; + + LocalStorage.get.mockReturnValue({ + data: { foo: 'bar' }, + timestamp: twoHoursAgo, + }); + + // Set expiry to 1 hour + const result = getFromCache('test-key', 60 * 60 * 1000); + + expect(result).toBeNull(); + expect(LocalStorage.get).toHaveBeenCalledWith('test-key'); + expect(LocalStorage.remove).toHaveBeenCalledWith('test-key'); + }); + + it('handles errors gracefully', () => { + LocalStorage.get.mockImplementation(() => { + throw new Error('Storage error'); + }); + + const result = getFromCache('test-key'); + + expect(result).toBeNull(); + }); + }); + + describe('setCache', () => { + it('stores data with timestamp', () => { + const data = { name: 'test' }; + const expectedCacheData = { + data, + timestamp: new Date(2023, 1, 1, 0, 0, 0).getTime(), + }; + + setCache('test-key', data); + + expect(LocalStorage.set).toHaveBeenCalledWith( + 'test-key', + expectedCacheData + ); + }); + + it('handles errors gracefully', () => { + LocalStorage.set.mockImplementation(() => { + throw new Error('Storage error'); + }); + + // Should not throw + expect(() => setCache('test-key', { foo: 'bar' })).not.toThrow(); + }); + }); + + describe('clearCache', () => { + it('removes cached data', () => { + clearCache('test-key'); + + expect(LocalStorage.remove).toHaveBeenCalledWith('test-key'); + }); + + it('handles errors gracefully', () => { + LocalStorage.remove.mockImplementation(() => { + throw new Error('Storage error'); + }); + + // Should not throw + expect(() => clearCache('test-key')).not.toThrow(); + }); + }); +}); diff --git a/app/javascript/widget/router.js b/app/javascript/widget/router.js index 71f82f7d1..e8230c2d9 100755 --- a/app/javascript/widget/router.js +++ b/app/javascript/widget/router.js @@ -1,11 +1,5 @@ import { createRouter, createWebHashHistory } from 'vue-router'; import ViewWithHeader from './components/layouts/ViewWithHeader.vue'; -import UnreadMessages from './views/UnreadMessages.vue'; -import Campaigns from './views/Campaigns.vue'; -import Home from './views/Home.vue'; -import PreChatForm from './views/PreChatForm.vue'; -import Messages from './views/Messages.vue'; -import ArticleViewer from './views/ArticleViewer.vue'; import store from './store'; const router = createRouter({ @@ -14,12 +8,12 @@ const router = createRouter({ { path: '/unread-messages', name: 'unread-messages', - component: UnreadMessages, + component: () => import('./views/UnreadMessages.vue'), }, { path: '/campaigns', name: 'campaigns', - component: Campaigns, + component: () => import('./views/Campaigns.vue'), }, { path: '/', @@ -28,22 +22,22 @@ const router = createRouter({ { path: '', name: 'home', - component: Home, + component: () => import('./views/Home.vue'), }, { path: '/prechat-form', name: 'prechat-form', - component: PreChatForm, + component: () => import('./views/PreChatForm.vue'), }, { path: '/messages', name: 'messages', - component: Messages, + component: () => import('./views/Messages.vue'), }, { path: '/article', name: 'article-viewer', - component: ArticleViewer, + component: () => import('./views/ArticleViewer.vue'), }, ], }, diff --git a/app/javascript/widget/store/modules/agent.js b/app/javascript/widget/store/modules/agent.js index 10f30cb02..2b9c38798 100644 --- a/app/javascript/widget/store/modules/agent.js +++ b/app/javascript/widget/store/modules/agent.js @@ -1,5 +1,6 @@ import { getAvailableAgents } from 'widget/api/agent'; import * as MutationHelpers from 'shared/helpers/vuex/mutationHelpers'; +import { getFromCache, setCache } from 'shared/helpers/cache'; const state = { records: [], @@ -15,11 +16,22 @@ export const getters = { $state.records.filter(agent => agent.availability_status === 'online'), }; +const CACHE_KEY_PREFIX = 'chatwoot_available_agents_'; + export const actions = { fetchAvailableAgents: async ({ commit }, websiteToken) => { try { + const cachedData = getFromCache(`${CACHE_KEY_PREFIX}${websiteToken}`); + if (cachedData) { + commit('setAgents', cachedData); + commit('setError', false); + commit('setHasFetched', true); + return; + } + const { data } = await getAvailableAgents(websiteToken); const { payload = [] } = data; + setCache(`${CACHE_KEY_PREFIX}${websiteToken}`, payload); commit('setAgents', payload); commit('setError', false); commit('setHasFetched', true); diff --git a/app/javascript/widget/store/modules/articles.js b/app/javascript/widget/store/modules/articles.js index 0f858012b..27faa297b 100644 --- a/app/javascript/widget/store/modules/articles.js +++ b/app/javascript/widget/store/modules/articles.js @@ -1,4 +1,7 @@ import { getMostReadArticles } from 'widget/api/article'; +import { getFromCache, setCache } from 'shared/helpers/cache'; + +const CACHE_KEY_PREFIX = 'chatwoot_most_read_articles_'; const state = { records: [], @@ -20,9 +23,16 @@ export const actions = { commit('setError', false); try { + const cachedData = getFromCache(`${CACHE_KEY_PREFIX}${slug}_${locale}`); + if (cachedData) { + commit('setArticles', cachedData); + return; + } + const { data } = await getMostReadArticles(slug, locale); const { payload = [] } = data; + setCache(`${CACHE_KEY_PREFIX}${slug}_${locale}`, payload); if (payload.length) { commit('setArticles', payload); } diff --git a/app/javascript/widget/store/modules/campaign.js b/app/javascript/widget/store/modules/campaign.js index ad2822dd9..050189fe5 100644 --- a/app/javascript/widget/store/modules/campaign.js +++ b/app/javascript/widget/store/modules/campaign.js @@ -4,6 +4,10 @@ import { formatCampaigns, filterCampaigns, } from 'widget/helpers/campaignHelper'; +import { getFromCache, setCache } from 'shared/helpers/cache'; + +const CACHE_KEY_PREFIX = 'chatwoot_campaigns_'; + const state = { records: [], uiFlags: { @@ -41,7 +45,26 @@ export const actions = { { websiteToken, currentURL, isInBusinessHours } ) => { try { + // Cache for 1 hour + const CACHE_EXPIRY = 60 * 60 * 1000; + const cachedData = getFromCache( + `${CACHE_KEY_PREFIX}${websiteToken}`, + CACHE_EXPIRY + ); + if (cachedData) { + commit('setCampaigns', cachedData); + commit('setError', false); + resetCampaignTimers( + cachedData, + currentURL, + websiteToken, + isInBusinessHours + ); + return; + } + const { data: campaigns } = await getCampaigns(websiteToken); + setCache(`${CACHE_KEY_PREFIX}${websiteToken}`, campaigns); commit('setCampaigns', campaigns); commit('setError', false); resetCampaignTimers( diff --git a/app/javascript/widget/store/modules/specs/agent/actions.spec.js b/app/javascript/widget/store/modules/specs/agent/actions.spec.js index 6b0465593..489835589 100644 --- a/app/javascript/widget/store/modules/specs/agent/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/agent/actions.spec.js @@ -1,14 +1,60 @@ -import { API } from 'widget/helpers/axios'; import { actions } from '../../agent'; import { agents } from './data'; +import { getFromCache, setCache } from 'shared/helpers/cache'; +import { getAvailableAgents } from 'widget/api/agent'; -const commit = vi.fn(); +let commit = vi.fn(); vi.mock('widget/helpers/axios'); +vi.mock('widget/api/agent'); +vi.mock('shared/helpers/cache'); + describe('#actions', () => { describe('#fetchAvailableAgents', () => { + const websiteToken = 'test-token'; + + beforeEach(() => { + commit = vi.fn(); + vi.clearAllMocks(); + }); + + it('returns cached data if available', async () => { + getFromCache.mockReturnValue(agents); + await actions.fetchAvailableAgents({ commit }, websiteToken); + + expect(getFromCache).toHaveBeenCalledWith( + `chatwoot_available_agents_${websiteToken}` + ); + expect(getAvailableAgents).not.toHaveBeenCalled(); + expect(setCache).not.toHaveBeenCalled(); + expect(commit).toHaveBeenCalledWith('setAgents', agents); + expect(commit).toHaveBeenCalledWith('setError', false); + expect(commit).toHaveBeenCalledWith('setHasFetched', true); + }); + + it('fetches and caches data if no cache available', async () => { + getFromCache.mockReturnValue(null); + getAvailableAgents.mockReturnValue({ data: { payload: agents } }); + + await actions.fetchAvailableAgents({ commit }, websiteToken); + + expect(getFromCache).toHaveBeenCalledWith( + `chatwoot_available_agents_${websiteToken}` + ); + expect(getAvailableAgents).toHaveBeenCalledWith(websiteToken); + expect(setCache).toHaveBeenCalledWith( + `chatwoot_available_agents_${websiteToken}`, + agents + ); + expect(commit).toHaveBeenCalledWith('setAgents', agents); + expect(commit).toHaveBeenCalledWith('setError', false); + expect(commit).toHaveBeenCalledWith('setHasFetched', true); + }); + it('sends correct actions if API is success', async () => { - API.get.mockResolvedValue({ data: { payload: agents } }); + getFromCache.mockReturnValue(null); + + getAvailableAgents.mockReturnValue({ data: { payload: agents } }); await actions.fetchAvailableAgents({ commit }, 'Hi'); expect(commit.mock.calls).toEqual([ ['setAgents', agents], @@ -17,7 +63,11 @@ describe('#actions', () => { ]); }); it('sends correct actions if API is error', async () => { - API.get.mockRejectedValue({ message: 'Authentication required' }); + getFromCache.mockReturnValue(null); + + getAvailableAgents.mockRejectedValue({ + message: 'Authentication required', + }); await actions.fetchAvailableAgents({ commit }, 'Hi'); expect(commit.mock.calls).toEqual([ ['setError', true], diff --git a/app/javascript/widget/store/modules/specs/article/actions.spec.js b/app/javascript/widget/store/modules/specs/article/actions.spec.js deleted file mode 100644 index 514cd10d3..000000000 --- a/app/javascript/widget/store/modules/specs/article/actions.spec.js +++ /dev/null @@ -1,127 +0,0 @@ -import { mutations, actions, getters } from '../../articles'; // update this import path to your actual module location -import { getMostReadArticles } from 'widget/api/article'; - -vi.mock('widget/api/article'); - -describe('Vuex Articles Module', () => { - let state; - - beforeEach(() => { - state = { - records: [], - uiFlags: { - isError: false, - hasFetched: false, - isFetching: false, - }, - }; - }); - - describe('Mutations', () => { - it('sets articles correctly', () => { - const articles = [{ id: 1 }, { id: 2 }]; - mutations.setArticles(state, articles); - expect(state.records).toEqual(articles); - }); - - it('sets error flag correctly', () => { - mutations.setError(state, true); - expect(state.uiFlags.isError).toBe(true); - }); - - it('sets fetching state correctly', () => { - mutations.setIsFetching(state, true); - expect(state.uiFlags.isFetching).toBe(true); - }); - - it('does not mutate records when no articles are provided', () => { - const previousState = { ...state }; - mutations.setArticles(state, []); - expect(state.records).toEqual(previousState.records); - }); - - it('toggles the error state correctly', () => { - mutations.setError(state, true); - expect(state.uiFlags.isError).toBe(true); - mutations.setError(state, false); - expect(state.uiFlags.isError).toBe(false); - }); - - it('toggles the fetching state correctly', () => { - mutations.setIsFetching(state, true); - expect(state.uiFlags.isFetching).toBe(true); - mutations.setIsFetching(state, false); - expect(state.uiFlags.isFetching).toBe(false); - }); - }); - - describe('Actions', () => { - it('fetches articles correctly', async () => { - const commit = vi.fn(); - const articles = [{ id: 1 }, { id: 2 }]; - getMostReadArticles.mockResolvedValueOnce({ - data: { payload: articles }, - }); - - await actions.fetch({ commit }, { slug: 'slug', locale: 'en' }); - - expect(commit).toHaveBeenCalledWith('setIsFetching', true); - expect(commit).toHaveBeenCalledWith('setError', false); - expect(commit).toHaveBeenCalledWith('setArticles', articles); - expect(commit).toHaveBeenCalledWith('setIsFetching', false); - }); - - it('handles fetch error correctly', async () => { - const commit = vi.fn(); - getMostReadArticles.mockRejectedValueOnce(new Error('Error message')); - - await actions.fetch( - { commit }, - { websiteToken: 'token', slug: 'slug', locale: 'en' } - ); - - expect(commit).toHaveBeenCalledWith('setIsFetching', true); - expect(commit).toHaveBeenCalledWith('setError', true); - expect(commit).toHaveBeenCalledWith('setIsFetching', false); - }); - - it('does not mutate state when fetching returns an empty payload', async () => { - const commit = vi.fn(); - getMostReadArticles.mockResolvedValueOnce({ data: { payload: [] } }); - - await actions.fetch({ commit }, { slug: 'slug', locale: 'en' }); - - expect(commit).toHaveBeenCalledWith('setIsFetching', true); - expect(commit).toHaveBeenCalledWith('setError', false); - expect(commit).not.toHaveBeenCalledWith('setArticles', expect.any(Array)); - expect(commit).toHaveBeenCalledWith('setIsFetching', false); - }); - - it('sets error state when fetching fails', async () => { - const commit = vi.fn(); - getMostReadArticles.mockRejectedValueOnce(new Error('Network error')); - - await actions.fetch( - { commit }, - { websiteToken: 'token', slug: 'slug', locale: 'en' } - ); - - expect(commit).toHaveBeenCalledWith('setIsFetching', true); - expect(commit).toHaveBeenCalledWith('setError', true); - expect(commit).not.toHaveBeenCalledWith('setArticles', expect.any(Array)); - expect(commit).toHaveBeenCalledWith('setIsFetching', false); - }); - }); - - describe('Getters', () => { - it('returns uiFlags correctly', () => { - const result = getters.uiFlags(state); - expect(result).toEqual(state.uiFlags); - }); - - it('returns popularArticles correctly', () => { - const result = getters.popularArticles(state); - expect(result).toEqual(state.records); - }); - }); -}); diff --git a/app/javascript/widget/store/modules/specs/articles.spec.js b/app/javascript/widget/store/modules/specs/articles.spec.js new file mode 100644 index 000000000..6d9ef4932 --- /dev/null +++ b/app/javascript/widget/store/modules/specs/articles.spec.js @@ -0,0 +1,170 @@ +import { mutations, actions, getters } from '../articles'; +import { getMostReadArticles } from 'widget/api/article'; +import { getFromCache, setCache } from 'shared/helpers/cache'; + +vi.mock('widget/api/article'); +vi.mock('shared/helpers/cache'); + +describe('Vuex Articles Module', () => { + let state; + + beforeEach(() => { + state = { + records: [], + uiFlags: { + isError: false, + hasFetched: false, + isFetching: false, + }, + }; + }); + + describe('Mutations', () => { + it('sets articles correctly', () => { + const articles = [{ id: 1 }, { id: 2 }]; + mutations.setArticles(state, articles); + expect(state.records).toEqual(articles); + }); + + it('sets error flag correctly', () => { + mutations.setError(state, true); + expect(state.uiFlags.isError).toBe(true); + }); + + it('sets fetching state correctly', () => { + mutations.setIsFetching(state, true); + expect(state.uiFlags.isFetching).toBe(true); + }); + + it('does not mutate records when no articles are provided', () => { + const previousState = { ...state }; + mutations.setArticles(state, []); + expect(state.records).toEqual(previousState.records); + }); + + it('toggles the error state correctly', () => { + mutations.setError(state, true); + expect(state.uiFlags.isError).toBe(true); + mutations.setError(state, false); + expect(state.uiFlags.isError).toBe(false); + }); + + it('toggles the fetching state correctly', () => { + mutations.setIsFetching(state, true); + expect(state.uiFlags.isFetching).toBe(true); + mutations.setIsFetching(state, false); + expect(state.uiFlags.isFetching).toBe(false); + }); + }); + + describe('Actions', () => { + describe('#fetch', () => { + const slug = 'test-slug'; + const locale = 'en'; + const articles = [ + { id: 1, title: 'Test' }, + { id: 2, title: 'Test 2' }, + ]; + let commit; + + beforeEach(() => { + commit = vi.fn(); + vi.clearAllMocks(); + }); + + it('returns cached data if available', async () => { + getFromCache.mockReturnValue(articles); + + await actions.fetch({ commit }, { slug, locale }); + + expect(getFromCache).toHaveBeenCalledWith( + `chatwoot_most_read_articles_${slug}_${locale}` + ); + expect(getMostReadArticles).not.toHaveBeenCalled(); + expect(setCache).not.toHaveBeenCalled(); + expect(commit).toHaveBeenCalledWith('setArticles', articles); + expect(commit).toHaveBeenCalledWith('setError', false); + }); + + it('fetches and caches data if no cache available', async () => { + getFromCache.mockReturnValue(null); + getMostReadArticles.mockReturnValue({ data: { payload: articles } }); + + await actions.fetch({ commit }, { slug, locale }); + + expect(getFromCache).toHaveBeenCalledWith( + `chatwoot_most_read_articles_${slug}_${locale}` + ); + expect(getMostReadArticles).toHaveBeenCalledWith(slug, locale); + expect(setCache).toHaveBeenCalledWith( + `chatwoot_most_read_articles_${slug}_${locale}`, + articles + ); + expect(commit).toHaveBeenCalledWith('setArticles', articles); + expect(commit).toHaveBeenCalledWith('setError', false); + }); + + it('handles API errors correctly', async () => { + getFromCache.mockReturnValue(null); + getMostReadArticles.mockRejectedValue(new Error('API Error')); + + await actions.fetch({ commit }, { slug, locale }); + + expect(commit).toHaveBeenCalledWith('setError', true); + expect(commit).toHaveBeenCalledWith('setIsFetching', false); + }); + + it('does not mutate state when fetching returns an empty payload', async () => { + getFromCache.mockReturnValue(null); + getMostReadArticles.mockReturnValue({ data: { payload: [] } }); + + await actions.fetch({ commit }, { slug, locale }); + + expect(commit).toHaveBeenCalledWith('setIsFetching', true); + expect(commit).toHaveBeenCalledWith('setError', false); + expect(commit).not.toHaveBeenCalledWith( + 'setArticles', + expect.any(Array) + ); + expect(commit).toHaveBeenCalledWith('setIsFetching', false); + }); + + it('sets loading state during fetch', async () => { + getFromCache.mockReturnValue(null); + getMostReadArticles.mockReturnValue({ data: { payload: articles } }); + + await actions.fetch({ commit }, { slug, locale }); + + expect(commit).toHaveBeenCalledWith('setIsFetching', true); + expect(commit).toHaveBeenCalledWith('setIsFetching', false); + }); + }); + + it('sets error state when fetching fails', async () => { + const commit = vi.fn(); + getMostReadArticles.mockRejectedValueOnce(new Error('Network error')); + + await actions.fetch( + { commit }, + { websiteToken: 'token', slug: 'slug', locale: 'en' } + ); + + expect(commit).toHaveBeenCalledWith('setIsFetching', true); + expect(commit).toHaveBeenCalledWith('setError', true); + expect(commit).not.toHaveBeenCalledWith('setArticles', expect.any(Array)); + expect(commit).toHaveBeenCalledWith('setIsFetching', false); + }); + }); + + describe('Getters', () => { + it('returns uiFlags correctly', () => { + const result = getters.uiFlags(state); + expect(result).toEqual(state.uiFlags); + }); + + it('returns popularArticles correctly', () => { + const result = getters.popularArticles(state); + expect(result).toEqual(state.records); + }); + }); +}); 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 c24490171..793ef6054 100644 --- a/app/javascript/widget/store/modules/specs/campaign/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/campaign/actions.spec.js @@ -1,10 +1,12 @@ import { API } from 'widget/helpers/axios'; import { actions } from '../../campaign'; import { campaigns } from './data'; +import { getFromCache, setCache } from 'shared/helpers/cache'; const commit = vi.fn(); const dispatch = vi.fn(); vi.mock('widget/helpers/axios'); +vi.mock('shared/helpers/cache'); import campaignTimer from 'widget/helpers/campaignTimer'; vi.mock('widget/helpers/campaignTimer', () => ({ @@ -15,8 +17,17 @@ vi.mock('widget/helpers/campaignTimer', () => ({ describe('#actions', () => { describe('#fetchCampaigns', () => { - it('sends correct actions if API is success', async () => { - API.get.mockResolvedValue({ data: campaigns }); + beforeEach(() => { + commit.mockClear(); + getFromCache.mockClear(); + setCache.mockClear(); + API.get.mockClear(); + campaignTimer.initTimers.mockClear(); + }); + + it('uses cached data when available', async () => { + getFromCache.mockReturnValue(campaigns); + await actions.fetchCampaigns( { commit }, { @@ -25,6 +36,54 @@ describe('#actions', () => { isInBusinessHours: true, } ); + + expect(getFromCache).toHaveBeenCalledWith( + 'chatwoot_campaigns_XDsafmADasd', + 60 * 60 * 1000 + ); + expect(API.get).not.toHaveBeenCalled(); + expect(setCache).not.toHaveBeenCalled(); + expect(commit.mock.calls).toEqual([ + ['setCampaigns', campaigns], + ['setError', false], + ]); + expect(campaignTimer.initTimers).toHaveBeenCalledWith( + { + campaigns: [ + { + id: 11, + timeOnPage: '20', + url: 'https://chatwoot.com', + triggerOnlyDuringBusinessHours: false, + }, + ], + }, + 'XDsafmADasd' + ); + }); + + it('fetches and caches data when cache is not available', async () => { + getFromCache.mockReturnValue(null); + API.get.mockResolvedValue({ data: campaigns }); + + await actions.fetchCampaigns( + { commit }, + { + websiteToken: 'XDsafmADasd', + currentURL: 'https://chatwoot.com', + isInBusinessHours: true, + } + ); + + expect(getFromCache).toHaveBeenCalledWith( + 'chatwoot_campaigns_XDsafmADasd', + 60 * 60 * 1000 + ); + expect(API.get).toHaveBeenCalled(); + expect(setCache).toHaveBeenCalledWith( + 'chatwoot_campaigns_XDsafmADasd', + campaigns + ); expect(commit.mock.calls).toEqual([ ['setCampaigns', campaigns], ['setError', false], @@ -43,7 +102,9 @@ describe('#actions', () => { 'XDsafmADasd' ); }); + it('sends correct actions if API is error', async () => { + getFromCache.mockReturnValue(null); API.get.mockRejectedValue({ message: 'Authentication required' }); await actions.fetchCampaigns( { commit }, diff --git a/app/services/base/send_on_channel_service.rb b/app/services/base/send_on_channel_service.rb index 58f225ef0..298f60962 100644 --- a/app/services/base/send_on_channel_service.rb +++ b/app/services/base/send_on_channel_service.rb @@ -12,10 +12,8 @@ class Base::SendOnChannelService def perform validate_target_channel - return unless outgoing_message? return if invalid_message? - return if invalid_source_id? perform_reply end @@ -51,29 +49,6 @@ class Base::SendOnChannelService message.private? || outgoing_message_originated_from_channel? end - def invalid_source_id? - return false unless channels_to_validate? - - return false if contact_inbox.source_id == expected_source_id - - message.update!(status: :failed, external_error: I18n.t('errors.channel_service.invalid_source_id')) - true - end - - def expected_source_id - ContactInbox::SourceIdService.new( - contact: contact, - channel_type: inbox.channel_type, - medium: inbox.channel.try(:medium) - ).generate - rescue ArgumentError - nil - end - - def channels_to_validate? - inbox.sms? || inbox.whatsapp? || inbox.email? || inbox.twilio? - end - def validate_target_channel raise 'Invalid channel service was called' if inbox.channel.class != channel_class end diff --git a/app/services/contact_inbox/source_id_service.rb b/app/services/contact_inbox/source_id_service.rb deleted file mode 100644 index 995bc37a9..000000000 --- a/app/services/contact_inbox/source_id_service.rb +++ /dev/null @@ -1,55 +0,0 @@ -class ContactInbox::SourceIdService - pattr_initialize [:contact!, :channel_type!, { medium: nil }] - - def generate - case channel_type - when 'Channel::TwilioSms' - twilio_source_id - when 'Channel::Whatsapp' - wa_source_id - when 'Channel::Email' - email_source_id - when 'Channel::Sms' - phone_source_id - when 'Channel::Api', 'Channel::WebWidget' - SecureRandom.uuid - else - raise ArgumentError, "Unsupported operation for this channel: #{channel_type}" - end - end - - private - - def email_source_id - raise ArgumentError, 'contact email required' unless contact.email - - contact.email - end - - def phone_source_id - raise ArgumentError, 'contact phone number required' unless contact.phone_number - - contact.phone_number - end - - def wa_source_id - raise ArgumentError, 'contact phone number required' unless contact.phone_number - - # whatsapp doesn't want the + in e164 format - contact.phone_number.delete('+').to_s - end - - def twilio_source_id - raise ArgumentError, 'contact phone number required' unless contact.phone_number - raise ArgumentError, 'medium required for Twilio channel' if medium.blank? - - case medium - when 'sms' - contact.phone_number - when 'whatsapp' - "whatsapp:#{contact.phone_number}" - else - raise ArgumentError, "Unsupported Twilio medium: #{medium}" - end - end -end diff --git a/config/locales/en.yml b/config/locales/en.yml index 27ba48cba..4a66a4bc2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -67,9 +67,6 @@ en: invalid_message_type: 'Invalid message type. Action not permitted' slack: invalid_channel_id: 'Invalid slack channel. Please try again' - channel_service: - invalid_source_id: "This conversation may have originally belonged to a different contact but is now showing here due to a merge or update. You won't be able to continue this conversation. Please create a new conversation to proceed." - inboxes: imap: socket_error: Please check the network connection, IMAP address and try again. diff --git a/spec/builders/contact_inbox_builder_spec.rb b/spec/builders/contact_inbox_builder_spec.rb index b6519f130..f0e3deb31 100644 --- a/spec/builders/contact_inbox_builder_spec.rb +++ b/spec/builders/contact_inbox_builder_spec.rb @@ -59,7 +59,7 @@ describe ContactInboxBuilder do contact: contact, inbox: twilio_inbox ).perform - end.to raise_error(ArgumentError, 'contact phone number required') + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') end end @@ -117,7 +117,7 @@ describe ContactInboxBuilder do contact: contact, inbox: twilio_inbox ).perform - end.to raise_error(ArgumentError, 'contact phone number required') + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') end end @@ -174,7 +174,7 @@ describe ContactInboxBuilder do contact: contact, inbox: whatsapp_inbox ).perform - end.to raise_error(ArgumentError, 'contact phone number required') + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') end end @@ -232,7 +232,7 @@ describe ContactInboxBuilder do contact: contact, inbox: sms_inbox ).perform - end.to raise_error(ArgumentError, 'contact phone number required') + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact phone number') end end @@ -290,7 +290,7 @@ describe ContactInboxBuilder do contact: contact, inbox: email_inbox ).perform - end.to raise_error(ArgumentError, 'contact email required') + end.to raise_error(ActionController::ParameterMissing, 'param is missing or the value is empty: contact email') end end diff --git a/spec/services/contact_inbox/source_id_service_spec.rb b/spec/services/contact_inbox/source_id_service_spec.rb deleted file mode 100644 index 94fe8a384..000000000 --- a/spec/services/contact_inbox/source_id_service_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -require 'rails_helper' - -RSpec.describe ContactInbox::SourceIdService do - let(:contact) { create(:contact, email: 'test@example.com', phone_number: '+1234567890') } - - describe '#generate' do - context 'when channel is TwilioSms' do - let(:channel_type) { 'Channel::TwilioSms' } - - context 'with SMS medium' do - subject { described_class.new(contact: contact, channel_type: channel_type, medium: 'sms') } - - it 'returns phone number as source id' do - expect(subject.generate).to eq(contact.phone_number) - end - end - - context 'with WhatsApp medium' do - subject { described_class.new(contact: contact, channel_type: channel_type, medium: 'whatsapp') } - - it 'returns whatsapp prefixed phone number as source id' do - expect(subject.generate).to eq("whatsapp:#{contact.phone_number}") - end - end - - context 'with invalid medium' do - subject { described_class.new(contact: contact, channel_type: channel_type, medium: 'invalid') } - - it 'raises ArgumentError' do - expect { subject.generate }.to raise_error(ArgumentError, 'Unsupported Twilio medium: invalid') - end - end - - context 'without medium' do - subject { described_class.new(contact: contact, channel_type: channel_type) } - - it 'raises ArgumentError' do - expect { subject.generate }.to raise_error(ArgumentError, 'medium required for Twilio channel') - end - end - end - - context 'when channel is Whatsapp' do - subject { described_class.new(contact: contact, channel_type: 'Channel::Whatsapp') } - - it 'returns phone number without + as source id' do - expect(subject.generate).to eq(contact.phone_number.delete('+')) - end - - context 'when contact has no phone number' do - let(:contact) { create(:contact, phone_number: nil) } - - it 'raises ArgumentError' do - expect { subject.generate }.to raise_error(ArgumentError, 'contact phone number required') - end - end - end - - context 'when channel is Email' do - subject { described_class.new(contact: contact, channel_type: 'Channel::Email') } - - it 'returns email as source id' do - expect(subject.generate).to eq(contact.email) - end - - context 'when contact has no email' do - let(:contact) { create(:contact, email: nil) } - - it 'raises ArgumentError' do - expect { subject.generate }.to raise_error(ArgumentError, 'contact email required') - end - end - end - - context 'when channel is SMS' do - subject { described_class.new(contact: contact, channel_type: 'Channel::Sms') } - - it 'returns phone number as source id' do - expect(subject.generate).to eq(contact.phone_number) - end - - context 'when contact has no phone number' do - let(:contact) { create(:contact, phone_number: nil) } - - it 'raises ArgumentError' do - expect { subject.generate }.to raise_error(ArgumentError, 'contact phone number required') - end - end - end - - context 'when channel is Api' do - subject { described_class.new(contact: contact, channel_type: 'Channel::Api') } - - it 'returns a UUID as source id' do - allow(SecureRandom).to receive(:uuid).and_return('uuid-123') - expect(subject.generate).to eq('uuid-123') - end - end - - context 'when channel is WebWidget' do - subject { described_class.new(contact: contact, channel_type: 'Channel::WebWidget') } - - it 'returns a UUID as source id' do - allow(SecureRandom).to receive(:uuid).and_return('uuid-123') - expect(subject.generate).to eq('uuid-123') - end - end - - context 'when channel is unsupported' do - subject { described_class.new(contact: contact, channel_type: 'Channel::Unknown') } - - it 'raises ArgumentError' do - expect { subject.generate }.to raise_error(ArgumentError, 'Unsupported operation for this channel: Channel::Unknown') - end - end - end -end diff --git a/spec/services/sms/send_on_sms_service_spec.rb b/spec/services/sms/send_on_sms_service_spec.rb index 52c5d89b1..d2dc1f24a 100644 --- a/spec/services/sms/send_on_sms_service_spec.rb +++ b/spec/services/sms/send_on_sms_service_spec.rb @@ -5,9 +5,8 @@ describe Sms::SendOnSmsService do context 'when a valid message' do let(:sms_request) { double } let!(:sms_channel) { create(:channel_sms) } - let!(:contact) { create(:contact, phone_number: '+123456789') } - let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: sms_channel.inbox, source_id: '+123456789') } - let!(:conversation) { create(:conversation, contact_inbox: contact_inbox, inbox: sms_channel.inbox, contact: contact) } + let!(:contact_inbox) { create(:contact_inbox, inbox: sms_channel.inbox, source_id: '+123456789') } + let!(:conversation) { create(:conversation, contact_inbox: contact_inbox, inbox: sms_channel.inbox) } it 'calls channel.send_message' do message = create(:message, message_type: :outgoing, content: 'test', diff --git a/spec/services/twilio/send_on_twilio_service_spec.rb b/spec/services/twilio/send_on_twilio_service_spec.rb index af74a46e7..beae54b57 100644 --- a/spec/services/twilio/send_on_twilio_service_spec.rb +++ b/spec/services/twilio/send_on_twilio_service_spec.rb @@ -13,11 +13,9 @@ describe Twilio::SendOnTwilioService do let!(:twilio_whatsapp) { create(:channel_twilio_sms, medium: :whatsapp, account: account) } let!(:twilio_inbox) { create(:inbox, channel: twilio_sms, account: account) } let!(:twilio_whatsapp_inbox) { create(:inbox, channel: twilio_whatsapp, account: account) } - let!(:contact) { create(:contact, account: account, phone_number: '+123456789') } - let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: twilio_inbox, source_id: '+123456789') } - let(:whatsapp_contact_inbox) { create(:contact_inbox, contact: contact, inbox: twilio_whatsapp_inbox, source_id: 'whatsapp:+123456789') } + let!(:contact) { create(:contact, account: account) } + let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: twilio_inbox) } let(:conversation) { create(:conversation, contact: contact, inbox: twilio_inbox, contact_inbox: contact_inbox) } - let(:whatsapp_conversation) { create(:conversation, contact: contact, inbox: twilio_whatsapp_inbox, contact_inbox: whatsapp_contact_inbox) } before do allow(Twilio::REST::Client).to receive(:new).and_return(twilio_client) @@ -73,7 +71,7 @@ describe Twilio::SendOnTwilioService do allow(message_record_double).to receive(:sid).and_return('1234') message = build( - :message, message_type: 'outgoing', inbox: twilio_whatsapp_inbox, account: account, conversation: whatsapp_conversation + :message, message_type: 'outgoing', inbox: twilio_whatsapp_inbox, account: account, conversation: conversation ) attachment = message.attachments.new(account_id: message.account_id, file_type: :image) attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png') diff --git a/spec/services/whatsapp/send_on_whatsapp_service_spec.rb b/spec/services/whatsapp/send_on_whatsapp_service_spec.rb index deb7d6a3f..88f3f7740 100644 --- a/spec/services/whatsapp/send_on_whatsapp_service_spec.rb +++ b/spec/services/whatsapp/send_on_whatsapp_service_spec.rb @@ -18,9 +18,8 @@ describe Whatsapp::SendOnWhatsappService do context 'when a valid message' do let(:whatsapp_request) { instance_double(HTTParty::Response) } let!(:whatsapp_channel) { create(:channel_whatsapp, sync_templates: false) } - let!(:contact) { create(:contact, phone_number: '+123456789') } - let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: whatsapp_channel.inbox, source_id: '123456789') } - let!(:conversation) { create(:conversation, contact: contact, contact_inbox: contact_inbox, inbox: whatsapp_channel.inbox) } + let!(:contact_inbox) { create(:contact_inbox, inbox: whatsapp_channel.inbox, source_id: '123456789') } + let!(:conversation) { create(:conversation, contact_inbox: contact_inbox, inbox: whatsapp_channel.inbox) } let(:api_key) { 'test_key' } let(:headers) { { 'D360-API-KEY' => api_key, 'Content-Type' => 'application/json' } } let(:template_body) do @@ -36,12 +35,14 @@ describe Whatsapp::SendOnWhatsappService do } end - let(:success_response) { { 'messages' => [{ 'id' => 'message-123456789' }] }.to_json } + let(:success_response) { { 'messages' => [{ 'id' => '123456789' }] }.to_json } it 'calls channel.send_message when with in 24 hour limit' do # to handle the case of 24 hour window limit. - create(:message, message_type: :incoming, content: 'test', conversation: conversation) - message = create(:message, message_type: :outgoing, content: 'test', conversation: conversation) + create(:message, message_type: :incoming, content: 'test', + conversation: conversation) + message = create(:message, message_type: :outgoing, content: 'test', + conversation: conversation) stub_request(:post, 'https://waba.360dialog.io/v1/messages') .with( @@ -51,7 +52,7 @@ describe Whatsapp::SendOnWhatsappService do .to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' }) described_class.new(message: message).perform - expect(message.reload.source_id).to eq('message-123456789') + expect(message.reload.source_id).to eq('123456789') end it 'calls channel.send_template when after 24 hour limit' do @@ -65,7 +66,7 @@ describe Whatsapp::SendOnWhatsappService do ).to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' }) described_class.new(message: message).perform - expect(message.reload.source_id).to eq('message-123456789') + expect(message.reload.source_id).to eq('123456789') end it 'calls channel.send_template if template_params are present' do @@ -78,7 +79,7 @@ describe Whatsapp::SendOnWhatsappService do ).to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' }) described_class.new(message: message).perform - expect(message.reload.source_id).to eq('message-123456789') + expect(message.reload.source_id).to eq('123456789') end it 'calls channel.send_template when template has regexp characters' do @@ -105,18 +106,7 @@ describe Whatsapp::SendOnWhatsappService do ).to_return(status: 200, body: success_response, headers: { 'content-type' => 'application/json' }) described_class.new(message: message).perform - expect(message.reload.source_id).to eq('message-123456789') - end - - context 'when source_id validation is required' do - let(:message) { create(:message, conversation: conversation, message_type: :outgoing) } - - it 'marks message as failed when source_ids do not match' do - contact_inbox.update!(source_id: '1234567890') - described_class.new(message: message).perform - expect(message.reload.status).to eq('failed') - expect(message.external_error).to include('This conversation may have originally belonged to a different contact') - end + expect(message.reload.source_id).to eq('123456789') end end end