From 07d19362d226c2306bed14bf293e3d45bc5ba624 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 24 Mar 2025 13:55:45 +0530 Subject: [PATCH 01/63] chore(deps): Bump nokogiri from 1.18.3 to 1.18.4 (#11153) Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.18.3 to 1.18.4.
Release notes

Sourced from nokogiri's releases.

v1.18.4 / 2025-03-14

Security

8f2263cef9953ce09bd5293d76c9bbd3013d2f94d1cca67783dfe6635c529deb
nokogiri-1.18.4-aarch64-linux-gnu.gem
4e231f8ba3128cfc2ef0cc0bdc807d7ce71fc62cb6a78216e817be8631fe6a96
nokogiri-1.18.4-aarch64-linux-musl.gem
73902663b23b1123282b9c0b6d9654b1fb286dfee8d65cb1f6029087b7f0d037
nokogiri-1.18.4-arm64-darwin.gem
cc2945e2c19560a61a97737e6bd3b329edb1f82ca204d46a18e5e98ad0a550a6
nokogiri-1.18.4-arm-linux-gnu.gem
4fb7f44de0cd85abfa869e4cfb619410da174ebf9fbe26ae0caa65462b818bcb
nokogiri-1.18.4-arm-linux-musl.gem
bb7820521c1bbae1d3e0092ff03b27a8e700912b37d80f962b7e4567947a64ac
nokogiri-1.18.4.gem
cbc0bab72eb5a9573efa7b98351fdd44c609e8d4585456ca1be18db2b7764b64
nokogiri-1.18.4-java.gem
bd567cb509eb75de8f27ca6ecaf4a38bf0563482188991f9bcccccac9c3b9a2f
nokogiri-1.18.4-x64-mingw-ucrt.gem
e4776f58eea9b94d05caf8bf351e3c6aa1cce01edcc2ed530f3c302c13178965
nokogiri-1.18.4-x86_64-darwin.gem
b1c6407b346b88704e97a342a80acd4755175324e624da34d0c5cfdc8d34191e
nokogiri-1.18.4-x86_64-linux-gnu.gem
ea7c0356a70f3d2d0d76315b533877013d20368d5c9f437c38e0bd462c4844dc
nokogiri-1.18.4-x86_64-linux-musl.gem
Changelog

Sourced from nokogiri's changelog.

v1.18.4 / 2025-03-14

Security

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=nokogiri&package-manager=bundler&previous-version=1.18.3&new-version=1.18.4)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/chatwoot/chatwoot/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Shivam Mishra --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index bfe4b1970..9f1ee5eec 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -501,14 +501,14 @@ GEM newrelic_rpm (9.6.0) base64 nio4r (2.7.3) - nokogiri (1.18.3) + nokogiri (1.18.4) mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.18.3-arm64-darwin) + nokogiri (1.18.4-arm64-darwin) racc (~> 1.4) - nokogiri (1.18.3-x86_64-darwin) + nokogiri (1.18.4-x86_64-darwin) racc (~> 1.4) - nokogiri (1.18.3-x86_64-linux-gnu) + nokogiri (1.18.4-x86_64-linux-gnu) racc (~> 1.4) oauth (1.1.0) oauth-tty (~> 1.0, >= 1.0.1) From 41d6f9a200b5cef67a88b28d6f31bc239b5b8c39 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Tue, 25 Mar 2025 04:34:49 +0530 Subject: [PATCH 02/63] chore: Add cache to improve widget performance (#11163) - Add dynamic importing for routes. - Added caching for `campaign`, `articles` and `inbox_members` API end points. --------- Co-authored-by: Pranav --- app/builders/contact_inbox_builder.rb | 51 +++++- .../contacts/contact_inboxes_controller.rb | 2 - .../helpers/composeConversationHelper.js | 5 - .../specs/composeConversationHelper.spec.js | 19 +- app/javascript/shared/helpers/cache.js | 43 +++++ .../shared/helpers/specs/cache.spec.js | 136 ++++++++++++++ app/javascript/widget/router.js | 18 +- app/javascript/widget/store/modules/agent.js | 12 ++ .../widget/store/modules/articles.js | 10 ++ .../widget/store/modules/campaign.js | 23 +++ .../store/modules/specs/agent/actions.spec.js | 58 +++++- .../modules/specs/article/actions.spec.js | 127 ------------- .../store/modules/specs/articles.spec.js | 170 ++++++++++++++++++ .../modules/specs/campaign/actions.spec.js | 65 ++++++- app/services/base/send_on_channel_service.rb | 25 --- .../contact_inbox/source_id_service.rb | 55 ------ config/locales/en.yml | 3 - spec/builders/contact_inbox_builder_spec.rb | 10 +- .../contact_inbox/source_id_service_spec.rb | 117 ------------ spec/services/sms/send_on_sms_service_spec.rb | 5 +- .../twilio/send_on_twilio_service_spec.rb | 8 +- .../whatsapp/send_on_whatsapp_service_spec.rb | 32 ++-- 22 files changed, 589 insertions(+), 405 deletions(-) create mode 100644 app/javascript/shared/helpers/cache.js create mode 100644 app/javascript/shared/helpers/specs/cache.spec.js delete mode 100644 app/javascript/widget/store/modules/specs/article/actions.spec.js create mode 100644 app/javascript/widget/store/modules/specs/articles.spec.js delete mode 100644 app/services/contact_inbox/source_id_service.rb delete mode 100644 spec/services/contact_inbox/source_id_service_spec.rb 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 From 50efd28d16f35042758e195d99ed33804dc43fc3 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 25 Mar 2025 08:09:04 +0530 Subject: [PATCH 03/63] feat: Add support for frontend filtering of conversations (#11111) This pull request includes significant changes to the filtering logic for conversations in the frontend, here's a summary of the changes This includes adding a `matchesFilters` method that evaluates a conversation against the applied filters. It does so by first evaluating all the conditions, and later converting the results into a JSONLogic object that can be evaluated according to Postgres operator precedence ### Alignment Specs To ensure the frontend and backend implementations always align, we've added tests on both sides with same cases, for anyone fixing any regressions found in the frontend implementation, they need to ensure the existing tests always pass. Test Case | JavaScript Spec | Ruby Spec | Match? -- | -- | -- | -- **A AND B OR C** | Present | Present | Yes Matches when all conditions are true | Present | Present | Yes Matches when first condition is false but third is true | Present | Present | Yes Matches when first and second conditions are false but third is true | Present | Present | Yes Does not match when all conditions are false | Present | Present | Yes **A OR B AND C** | Present | Present | Yes Matches when first condition is true | Present | Present | Yes Matches when second and third conditions are true | Present | Present | Yes **A AND B OR C AND D** | Present | Present | Yes Matches when first two conditions are true | Present | Present | Yes Matches when last two conditions are true | Present | Present | Yes **Mixed Operators (A AND (B OR C) AND D)** | Present | Present | Yes Matches when all conditions in the chain are true | Present | Present | Yes Does not match when the last condition is false | Present | Present | Yes --------- Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Co-authored-by: Muhsin Keloth Co-authored-by: Pranav --- .../dashboard/components/ChatList.vue | 17 +- .../conversation/advancedFilterItems/index.js | 8 + .../dashboard/helper/customViewsHelper.js | 34 +- .../store/modules/conversations/getters.js | 10 + .../conversations/helpers/filterHelpers.js | 356 +++++ .../helpers/specs/filterHelpers.spec.js | 1302 +++++++++++++++++ package.json | 1 + pnpm-lock.yaml | 8 + .../conversations/filter_service_spec.rb | 237 +++ 9 files changed, 1961 insertions(+), 12 deletions(-) create mode 100644 app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js create mode 100644 app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js diff --git a/app/javascript/dashboard/components/ChatList.vue b/app/javascript/dashboard/components/ChatList.vue index a0b86176d..dc18ddd66 100644 --- a/app/javascript/dashboard/components/ChatList.vue +++ b/app/javascript/dashboard/components/ChatList.vue @@ -61,6 +61,7 @@ import { getUserPermissions, filterItemsByPermission, } from 'dashboard/helper/permissionsHelper.js'; +import { matchesFilters } from '../store/modules/conversations/helpers/filterHelpers'; import { CONVERSATION_EVENTS } from '../helper/AnalyticsHelper/events'; import { ASSIGNEE_TYPE_TAB_PERMISSIONS } from 'dashboard/constants/permissions.js'; @@ -105,7 +106,7 @@ const advancedFilterTypes = ref( ); const currentUser = useMapGetter('getCurrentUser'); -const chatLists = useMapGetter('getAllConversations'); +const chatLists = useMapGetter('getFilteredConversations'); const mineChatsList = useMapGetter('getMineChats'); const allChatList = useMapGetter('getAllStatusChats'); const unAssignedChatsList = useMapGetter('getUnAssignedChats'); @@ -324,6 +325,14 @@ const conversationList = computed(() => { } else { localConversationList = [...chatLists.value]; } + + if (activeFolder.value) { + const { payload } = activeFolder.value.query; + localConversationList = localConversationList.filter(conversation => { + return matchesFilters(conversation, payload); + }); + } + return localConversationList; }); @@ -460,6 +469,12 @@ function setParamsForEditFolderModal() { campaigns: campaigns.value, languages: languages, countries: countries, + priority: [ + { id: 'low', name: t('CONVERSATION.PRIORITY.OPTIONS.LOW') }, + { id: 'medium', name: t('CONVERSATION.PRIORITY.OPTIONS.MEDIUM') }, + { id: 'high', name: t('CONVERSATION.PRIORITY.OPTIONS.HIGH') }, + { id: 'urgent', name: t('CONVERSATION.PRIORITY.OPTIONS.URGENT') }, + ], filterTypes: advancedFilterTypes.value, allCustomAttributes: conversationCustomAttributes.value, }; diff --git a/app/javascript/dashboard/components/widgets/conversation/advancedFilterItems/index.js b/app/javascript/dashboard/components/widgets/conversation/advancedFilterItems/index.js index a3e8e2c10..08b456002 100644 --- a/app/javascript/dashboard/components/widgets/conversation/advancedFilterItems/index.js +++ b/app/javascript/dashboard/components/widgets/conversation/advancedFilterItems/index.js @@ -22,6 +22,14 @@ const filterTypes = [ filterOperators: OPERATOR_TYPES_2, attributeModel: 'standard', }, + { + attributeKey: 'priority', + attributeI18nKey: 'PRIORITY', + inputType: 'multi_select', + dataType: 'text', + filterOperators: OPERATOR_TYPES_1, + attributeModel: 'standard', + }, { attributeKey: 'inbox_id', attributeI18nKey: 'INBOX_NAME', diff --git a/app/javascript/dashboard/helper/customViewsHelper.js b/app/javascript/dashboard/helper/customViewsHelper.js index f3ed8a3a7..c837ce613 100644 --- a/app/javascript/dashboard/helper/customViewsHelper.js +++ b/app/javascript/dashboard/helper/customViewsHelper.js @@ -21,6 +21,7 @@ export const getAttributeInputType = (key, allCustomAttributes) => { const customAttribute = allCustomAttributes.find( attr => attr.attribute_key === key ); + const { attribute_display_type } = customAttribute; const filterInputTypes = generateCustomAttributesInputType( attribute_display_type @@ -68,10 +69,22 @@ const getValuesForCountries = (values, countries) => { })); }; +const getValuesForPriority = (values, priority) => { + return priority.filter(option => values.includes(option.id)); +}; + export const getValuesForFilter = (filter, params) => { const { attribute_key, values } = filter; - const { languages, countries, agents, inboxes, teams, campaigns, labels } = - params; + const { + languages, + countries, + agents, + inboxes, + teams, + campaigns, + labels, + priority, + } = params; switch (attribute_key) { case 'status': return getValuesForStatus(values); @@ -83,15 +96,14 @@ export const getValuesForFilter = (filter, params) => { return getValuesName(values, teams, 'id', 'name'); case 'campaign_id': return getValuesName(values, campaigns, 'id', 'title'); - case 'labels': { + case 'labels': return getValuesForLabels(values, labels); - } - case 'browser_language': { + case 'priority': + return getValuesForPriority(values, priority); + case 'browser_language': return getValuesForLanguages(values, languages); - } - case 'country_code': { + case 'country_code': return getValuesForCountries(values, countries); - } default: return { id: values[0], name: values[0] }; } @@ -100,9 +112,9 @@ export const getValuesForFilter = (filter, params) => { export const generateValuesForEditCustomViews = (filter, params) => { const { attribute_key, filter_operator, values } = filter; const { filterTypes, allCustomAttributes } = params; - const inboxType = getInputType(attribute_key, filter_operator, filterTypes); + const inputType = getInputType(attribute_key, filter_operator, filterTypes); - if (inboxType === undefined) { + if (inputType === undefined) { const filterInputTypes = getAttributeInputType( attribute_key, allCustomAttributes @@ -112,7 +124,7 @@ export const generateValuesForEditCustomViews = (filter, params) => { : { id: values[0], name: values[0] }; } - return inboxType === 'multi_select' || inboxType === 'search_select' + return inputType === 'multi_select' || inputType === 'search_select' ? getValuesForFilter(filter, params) : values[0].toString(); }; diff --git a/app/javascript/dashboard/store/modules/conversations/getters.js b/app/javascript/dashboard/store/modules/conversations/getters.js index 9329ef5dc..33f140fad 100644 --- a/app/javascript/dashboard/store/modules/conversations/getters.js +++ b/app/javascript/dashboard/store/modules/conversations/getters.js @@ -1,6 +1,7 @@ import { MESSAGE_TYPE } from 'shared/constants/messages'; import { applyPageFilters, sortComparator } from './helpers'; import filterQueryGenerator from 'dashboard/helper/filterQueryGenerator'; +import { matchesFilters } from './helpers/filterHelpers'; import camelcaseKeys from 'camelcase-keys'; export const getSelectedChatConversation = ({ @@ -13,6 +14,15 @@ const getters = { getAllConversations: ({ allConversations, chatSortFilter: sortKey }) => { return allConversations.sort((a, b) => sortComparator(a, b, sortKey)); }, + getFilteredConversations: ({ + allConversations, + chatSortFilter, + appliedFilters, + }) => { + return allConversations + .filter(conversation => matchesFilters(conversation, appliedFilters)) + .sort((a, b) => sortComparator(a, b, chatSortFilter)); + }, getSelectedChat: ({ selectedChatId, allConversations }) => { const selectedChat = allConversations.find( conversation => conversation.id === selectedChatId diff --git a/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js b/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js new file mode 100644 index 000000000..2591c2c01 --- /dev/null +++ b/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js @@ -0,0 +1,356 @@ +/** + * Conversation Filter Helpers + * --------------------------- + * This file contains helper functions for filtering conversations in the frontend. + * The filtering logic is designed to align with the backend SQL behavior to ensure + * consistent results across the application. + * + * Key components: + * 1. getValueFromConversation: Retrieves values from conversation objects, handling + * both top-level properties and nested attributes. + * 2. matchesCondition: Evaluates a single filter condition against a value. + * 3. matchesFilters: Evaluates a complete filter chain against a conversation. + * 4. buildJsonLogicRule: Transforms evaluated filters into a JSON Logic frule that + * respects SQL-like operator precedence. + * + * Filter Structure: + * ----------------- + * Each filter has the following structure: + * { + * attributeKey: 'status', // The attribute to filter on + * filterOperator: 'equal_to', // The operator to use (equal_to, contains, etc.) + * values: ['open'], // The values to compare against + * queryOperator: 'and' // How this filter connects to the next one (and/or) + * } + * + * Operator Precedence: + * -------------------- + * The filter evaluation respects SQL-like operator precedence using JSON Logic: + * https://www.postgresql.org/docs/17/sql-syntax-lexical.html#SQL-PRECEDENCE + * 1. First evaluates individual conditions + * 2. Then applies AND operators (groups consecutive AND conditions) + * 3. Finally applies OR operators (connects AND groups with OR operations) + * + * This means that a filter chain like "A AND B OR C" is evaluated as "(A AND B) OR C", + * and "A OR B AND C" is evaluated as "A OR (B AND C)". + * + * The implementation uses json-logic-js to apply these rules. The JsonLogic format is designed + * to allow you to share rules (logic) between front-end and back-end code + * Here we use json-logic-js to transform filter conditions into a nested JSON Logic structure that preserves proper + * operator precedence, effectively mimicking SQL-like operator precedence. + * + * Conversation Object Structure: + * ----------------------------- + * The conversation object can have: + * 1. Top-level properties (status, priority, display_id, etc.) + * 2. Nested properties in additional_attributes (browser_language, referer, etc.) + * 3. Nested properties in custom_attributes (conversation_type, etc.) + */ +import jsonLogic from 'json-logic-js'; + +/** + * Gets a value from a conversation based on the attribute key + * @param {Object} conversation - The conversation object + * @param {String} attributeKey - The attribute key to get the value for + * @returns {*} - The value of the attribute + * + * This function handles various attribute locations: + * 1. Direct properties on the conversation object (status, priority, etc.) + * 2. Properties in conversation.additional_attributes (browser_language, referer, etc.) + * 3. Properties in conversation.custom_attributes (conversation_type, etc.) + */ +const getValueFromConversation = (conversation, attributeKey) => { + switch (attributeKey) { + case 'status': + case 'priority': + case 'display_id': + case 'labels': + case 'created_at': + case 'last_activity_at': + return conversation[attributeKey]; + case 'assignee_id': + return conversation.meta?.assignee?.id; + case 'inbox_id': + return conversation.inbox_id; + case 'team_id': + return conversation.meta?.team?.id; + case 'browser_language': + case 'country_code': + case 'referer': + return conversation.additional_attributes?.[attributeKey]; + default: + // Check if it's a custom attribute + if ( + conversation.custom_attributes && + conversation.custom_attributes[attributeKey] + ) { + return conversation.custom_attributes[attributeKey]; + } + return null; + } +}; + +/** + * Resolves the value from an input candidate + * @param {*} candidate - The input value to resolve + * @returns {*} - If the candidate is an object with an id property, returns the id; + * otherwise returns the candidate unchanged + * + * This helper function is used to normalize values, particularly when dealing with + * objects that represent entities like users, teams, or inboxes where we want to + * compare by ID rather than by the whole object. + */ +const resolveValue = candidate => { + if ( + typeof candidate === 'object' && + candidate !== null && + 'id' in candidate + ) { + return candidate.id; + } + + return candidate; +}; + +/** + * Checks if two values are equal in the context of filtering + * @param {*} filterValue - The filterValue value + * @param {*} conversationValue - The conversationValue value + * @returns {Boolean} - Returns true if the values are considered equal according to filtering rules + * + * This function handles various equality scenarios: + * 1. When both values are arrays: checks if all items in filterValue exist in conversationValue + * 2. When filterValue is an array but conversationValue is not: checks if conversationValue is included in filterValue + * 3. Otherwise: performs strict equality comparison + */ +const equalTo = (filterValue, conversationValue) => { + if (Array.isArray(filterValue)) { + if (filterValue.includes('all')) return true; + if (filterValue === 'all') return true; + + if (Array.isArray(conversationValue)) { + // For array values like labels, check if any of the filter values exist in the array + return filterValue.every(val => conversationValue.includes(val)); + } + + if (!Array.isArray(conversationValue)) { + return filterValue.includes(conversationValue); + } + } + + return conversationValue === filterValue; +}; + +/** + * Checks if the filterValue value is contained within the conversationValue value + * @param {*} filterValue - The value to look for + * @param {*} conversationValue - The value to search within + * @returns {Boolean} - Returns true if filterValue is contained within conversationValue + * + * This function performs case-insensitive string containment checks. + * It only works with string values and returns false for non-string types. + */ +const contains = (filterValue, conversationValue) => { + if (typeof conversationValue === 'string') { + return conversationValue.toLowerCase().includes(filterValue.toLowerCase()); + } + return false; +}; + +/** + * Checks if a value matches a filter condition + * @param {*} conversationValue - The value to check + * @param {Object} filter - The filter condition + * @returns {Boolean} - Returns true if the value matches the filter + */ +const matchesCondition = (conversationValue, filter) => { + const { filter_operator: filterOperator, values } = filter; + + // Handle null/undefined values + if (conversationValue === null || conversationValue === undefined) { + return filterOperator === 'is_not_present'; + } + + const filterValue = Array.isArray(values) + ? values.map(resolveValue) + : resolveValue(values); + + switch (filterOperator) { + case 'equal_to': + return equalTo(filterValue, conversationValue); + + case 'not_equal_to': + return !equalTo(filterValue, conversationValue); + + case 'contains': + return contains(filterValue, conversationValue); + + case 'does_not_contain': + return !contains(filterValue, conversationValue); + + case 'is_present': + return true; // We already handled null/undefined above + + case 'is_not_present': + return false; // We already handled null/undefined above + + case 'is_greater_than': + return new Date(conversationValue) > new Date(filterValue); + + case 'is_less_than': + return new Date(conversationValue) < new Date(filterValue); + + case 'days_before': { + const today = new Date(); + const daysInMilliseconds = filterValue * 24 * 60 * 60 * 1000; + const targetDate = new Date(today.getTime() - daysInMilliseconds); + return conversationValue < targetDate.getTime(); + } + + default: + return false; + } +}; + +/** + * Converts an array of evaluated filters into a JSON Logic rule + * that respects SQL-like operator precedence (AND before OR) + * + * This function transforms the linear sequence of filter results and operators + * into a nested JSON Logic structure that correctly implements SQL-like precedence: + * - AND operators are evaluated before OR operators + * - Consecutive AND conditions are grouped together + * - These AND groups are then connected with OR operators + * + * For example: + * - "A AND B AND C" becomes { "and": [A, B, C] } + * - "A OR B OR C" becomes { "or": [A, B, C] } + * - "A AND B OR C" becomes { "or": [{ "and": [A, B] }, C] } + * - "A OR B AND C" becomes { "or": [A, { "and": [B, C] }] } + * + * FILTER CHAIN: A --AND--> B --OR--> C --AND--> D --AND--> E --OR--> F + * | | | | | | + * v v v v v v + * EVALUATED: true false true false true false + * \ / \ \ / / + * \ / \ \ / / + * \ / \ \ / / + * \ / \ \ / / + * \ / \ \ / / + * AND GROUPS: [true,false] [true,false,true] [false] + * | | | + * v v v + * JSON LOGIC: {"and":[true,false]} {"and":[true,false,true]} false + * \ | / + * \ | / + * \ | / + * \ | / + * \ | / + * FINAL RULE: {"or":[{"and":[true,false]},{"and":[true,false,true]},false]} + * + * { + * "or": [ + * { "and": [true, false] }, + * { "and": [true, false, true] }, + * { "and": [false] } + * ] + * } + * @param {Array} evaluatedFilters - Array of evaluated filter conditions with results and operators + * @returns {Object} - JSON Logic rule + */ +const buildJsonLogicRule = evaluatedFilters => { + // Step 1: Group consecutive AND conditions into logical units + // This implements the higher precedence of AND over OR + const andGroups = []; + let currentAndGroup = [evaluatedFilters[0].result]; + + for (let i = 0; i < evaluatedFilters.length - 1; i += 1) { + if (evaluatedFilters[i].operator === 'and') { + // When we see an AND operator, we add the next filter to the current AND group + // This builds up chains of AND conditions that will be evaluated together + currentAndGroup.push(evaluatedFilters[i + 1].result); + } else { + // When we see an OR operator, it marks the boundary between AND groups + // We finalize the current AND group and start a new one + + // If the AND group has only one item, don't wrap it in an "and" operator + // Otherwise, create a proper "and" JSON Logic expression + andGroups.push( + currentAndGroup.length === 1 + ? currentAndGroup[0] // Single item doesn't need an "and" wrapper + : { and: currentAndGroup } // Multiple items need to be AND-ed together + ); + + // Start a new AND group with the next filter's result + currentAndGroup = [evaluatedFilters[i + 1].result]; + } + } + + // Step 2: Add the final AND group that wasn't followed by an OR + if (currentAndGroup.length > 0) { + andGroups.push( + currentAndGroup.length === 1 + ? currentAndGroup[0] // Single item doesn't need an "and" wrapper + : { and: currentAndGroup } // Multiple items need to be AND-ed together + ); + } + + // Step 3: Combine all AND groups with OR operators + // If we have multiple AND groups, they are separated by OR operators + // in the original filter chain, so we combine them with an "or" operation + if (andGroups.length > 1) { + return { or: andGroups }; + } + + // If there's only one AND group (which might be a single condition + // or multiple AND-ed conditions), just return it directly + return andGroups[0]; +}; + +/** + * Evaluates each filter against the conversation and prepares the results array + * @param {Object} conversation - The conversation to evaluate + * @param {Array} filters - Filters to apply + * @returns {Array} - Array of evaluated filter results with operators + */ +const evaluateFilters = (conversation, filters) => { + return filters.map((filter, index) => { + const value = getValueFromConversation(conversation, filter.attribute_key); + const result = matchesCondition(value, filter); + + // This part determines the logical operator that connects this filter to the next one: + // - If this is not the last filter (index < filters.length - 1), use the filter's query_operator + // or default to 'and' if query_operator is not specified + // - If this is the last filter, set operator to null since there's no next filter to connect to + const isLastFilter = index === filters.length - 1; + const operator = isLastFilter ? null : filter.query_operator || 'and'; + + return { result, operator }; + }); +}; + +/** + * Checks if a conversation matches the given filters + * @param {Object} conversation - The conversation object to check + * @param {Array} filters - Array of filter conditions + * @returns {Boolean} - Returns true if conversation matches filters, false otherwise + */ +export const matchesFilters = (conversation, filters) => { + // If no filters, return true + if (!filters || filters.length === 0) { + return true; + } + + // Handle single filter case + if (filters.length === 1) { + const value = getValueFromConversation( + conversation, + filters[0].attribute_key + ); + return matchesCondition(value, filters[0]); + } + + // Evaluate all conditions and prepare for jsonLogic + const evaluatedFilters = evaluateFilters(conversation, filters); + return jsonLogic.apply(buildJsonLogicRule(evaluatedFilters)); +}; diff --git a/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js b/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js new file mode 100644 index 000000000..6d709f085 --- /dev/null +++ b/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js @@ -0,0 +1,1302 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { matchesFilters } from '../filterHelpers'; + +// SAMPLE PAYLOAD +// +// { +// attribute_key: 'status', +// attribute_model: 'standard', +// filter_operator: 'equal_to', +// values: [ +// { +// id: 'open', +// name: 'Open', +// }, +// { +// id: 'resolved', +// name: 'Resolved', +// }, +// { +// id: 'pending', +// name: 'Pending', +// }, +// { +// id: 'snoozed', +// name: 'Snoozed', +// }, +// { +// id: 'all', +// name: 'All', +// }, +// ], +// query_operator: 'and', +// custom_attribute_type: '', +// }, +// { +// attribute_key: 'priority', +// filter_operator: 'equal_to', +// values: [ +// { +// id: 'low', +// name: 'Low', +// }, +// { +// id: 'medium', +// name: 'Medium', +// }, +// { +// id: 'high', +// name: 'High', +// }, +// ], +// query_operator: 'and', +// }, +// { +// attribute_key: 'assignee_id', +// filter_operator: 'equal_to', +// values: { +// id: 12345, +// name: 'Agent Name', +// }, +// query_operator: 'and', +// }, +// { +// attribute_key: 'inbox_id', +// filter_operator: 'equal_to', +// values: { +// id: 37, +// name: 'Support Inbox', +// }, +// query_operator: 'and', +// }, +// { +// attribute_key: 'team_id', +// filter_operator: 'equal_to', +// values: { +// id: 220, +// name: 'support-team', +// }, +// query_operator: 'and', +// }, +// { +// attribute_key: 'created_at', +// filter_operator: 'is_greater_than', +// values: '2023-01-20', +// query_operator: 'and', +// }, +// { +// attribute_key: 'last_activity_at', +// filter_operator: 'days_before', +// values: '998', +// query_operator: 'and', +// }, + +describe('filterHelpers', () => { + describe('#matchesFilters', () => { + it('returns true by default when no filters are provided', () => { + const conversation = {}; + const filters = []; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Standard attribute tests - status + it('should match conversation with equal_to operator for status', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with equal_to operator for status "all"', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'all', name: 'all' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with not_equal_to operator for status "all"', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'not_equal_to', + values: [{ id: 'all', name: 'all' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should not match conversation with not_equal_to operator for status', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'not_equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + // Standard attribute tests - assignee_id + it('should match conversation with equal_to operator for assignee_id', () => { + const conversation = { meta: { assignee: { id: 1 } } }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'equal_to', + values: { id: 1, name: 'John Doe' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with not_equal_to operator for assignee_id', () => { + const conversation = { meta: { assignee: { id: 1 } } }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'not_equal_to', + values: { id: 2, name: 'Jane Smith' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with is_present operator for assignee_id', () => { + const conversation = { meta: { assignee: { id: 1 } } }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'is_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with is_not_present operator for assignee_id', () => { + const conversation = { meta: { assignee: null } }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'is_not_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with is_present operator when assignee is null', () => { + const conversation = { meta: { assignee: null } }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'is_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + // Standard attribute tests - priority + it('should match conversation with equal_to operator for priority', () => { + const conversation = { priority: 'urgent' }; + const filters = [ + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with not_equal_to operator for priority', () => { + const conversation = { priority: 'urgent' }; + const filters = [ + { + attribute_key: 'priority', + filter_operator: 'not_equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + // Text search tests - display_id + it('should match conversation with equal_to operator for display_id', () => { + const conversation = { display_id: '12345' }; + const filters = [ + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '12345', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with contains operator for display_id', () => { + const conversation = { display_id: '12345' }; + const filters = [ + { + attribute_key: 'display_id', + filter_operator: 'contains', + values: '234', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with does_not_contain operator for display_id', () => { + const conversation = { display_id: '12345' }; + const filters = [ + { + attribute_key: 'display_id', + filter_operator: 'does_not_contain', + values: '234', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should match conversation with does_not_contain operator when value is not present', () => { + const conversation = { display_id: '12345' }; + const filters = [ + { + attribute_key: 'display_id', + filter_operator: 'does_not_contain', + values: '678', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Array/List tests - labels + it('should match conversation with equal_to operator for labels', () => { + const conversation = { labels: ['support', 'urgent', 'new'] }; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with equal_to operator for labels when value is not present', () => { + const conversation = { labels: ['support', 'urgent', 'new'] }; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'equal_to', + values: [{ id: 'billing', name: 'Billing' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should match conversation with not_equal_to operator for labels when value is not present', () => { + const conversation = { labels: ['support', 'urgent', 'new'] }; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'not_equal_to', + values: [{ id: 'billing', name: 'Billing' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with is_present operator for labels', () => { + const conversation = { labels: ['support', 'urgent', 'new'] }; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'is_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with is_not_present operator for labels when labels is null', () => { + const conversation = { labels: null }; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'is_not_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with is_not_present operator for labels when labels is undefined', () => { + const conversation = {}; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'is_not_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Additional attributes tests + it('should match conversation with equal_to operator for browser_language', () => { + const conversation = { + additional_attributes: { browser_language: 'en-US' }, + }; + const filters = [ + { + attribute_key: 'browser_language', + filter_operator: 'equal_to', + values: 'en-US', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with contains operator for referer', () => { + const conversation = { + additional_attributes: { referer: 'https://www.chatwoot.com/pricing' }, + }; + const filters = [ + { + attribute_key: 'referer', + filter_operator: 'contains', + values: 'chatwoot', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with does_not_contain operator for referer', () => { + const conversation = { + additional_attributes: { referer: 'https://www.chatwoot.com/pricing' }, + }; + const filters = [ + { + attribute_key: 'referer', + filter_operator: 'does_not_contain', + values: 'chatwoot', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + // Date tests + it('should match conversation with is_greater_than operator for created_at', () => { + const conversation = { created_at: 1647777600000 }; // March 20, 2022 + const filters = [ + { + attribute_key: 'created_at', + filter_operator: 'is_greater_than', + values: '2022-03-19', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with is_greater_than operator for created_at when date is earlier', () => { + const conversation = { created_at: 1647691200000 }; // March 19, 2022 + const filters = [ + { + attribute_key: 'created_at', + filter_operator: 'is_greater_than', + values: '2022-03-20', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should match conversation with is_less_than operator for created_at', () => { + const conversation = { created_at: 1647777600000 }; // March 20, 2022 + const filters = [ + { + attribute_key: 'created_at', + filter_operator: 'is_less_than', + values: '2022-03-21', // March 21, 2022 + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + describe('days_before operator', () => { + beforeEach(() => { + // Set the date to March 25, 2022 + vi.useFakeTimers(); + vi.setSystemTime(new Date(2022, 2, 25)); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('should match conversation with days_before operator for created_at', () => { + const conversation = { created_at: 1647777600000 }; // March 20, 2022 + const filters = [ + { + attribute_key: 'created_at', + filter_operator: 'days_before', + values: '3', // 3 days before March 25 = March 22 + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + }); + + // Multiple filters tests + it('should match conversation with multiple filters combined with AND operator', () => { + const conversation = { + status: 'open', + priority: 'urgent', + meta: { assignee: { id: 1 } }, + }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'and', + }, + { + attribute_key: 'assignee_id', + filter_operator: 'equal_to', + values: { + id: 1, + name: 'Agent', + }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation when one filter in AND chain does not match', () => { + const conversation = { + status: 'open', + priority: 'urgent', + meta: { assignee: { id: 1 } }, + }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'low', name: 'Low' }], // This doesn't match + query_operator: 'and', + }, + { + attribute_key: 'assignee_id', + filter_operator: 'equal_to', + values: { + id: 1, + name: 'Agent', + }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should match conversation with multiple filters combined with OR operator', () => { + const conversation = { + status: 'open', + priority: 'low', + }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'closed', name: 'Closed' }], + query_operator: 'or', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'low', name: 'Low' }], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation when one filter in OR chain matches', () => { + const conversation = { + status: 'open', + priority: 'low', + }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], // This matches + query_operator: 'or', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], // This doesn't match + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with multiple status and priority', () => { + const conversation = { + status: 'open', + priority: 'high', + meta: { + assignee: { + id: 83235, + }, + }, + }; + + const filters = [ + { + values: ['open', 'resolved'], + attribute_key: 'status', + query_operator: 'and', + attribute_model: 'standard', + filter_operator: 'equal_to', + custom_attribute_type: '', + }, + { + values: [83235], + attribute_key: 'assignee_id', + query_operator: 'and', + filter_operator: 'equal_to', + }, + { + values: ['high', 'urgent'], + attribute_key: 'priority', + filter_operator: 'equal_to', + }, + ]; + + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Nested property tests + it('should match conversation with filter on nested property in meta', () => { + const conversation = { + meta: { + team: { + id: 5, + name: 'Support', + }, + }, + }; + const filters = [ + { + attribute_key: 'team_id', + filter_operator: 'equal_to', + values: { id: 5, name: 'Support' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Edge cases + it('should handle null values in conversation', () => { + const conversation = { + status: null, + priority: 'low', + }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'is_not_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should handle empty arrays in conversation', () => { + const conversation = { + labels: [], + }; + const filters = [ + { + attribute_key: 'labels', + filter_operator: 'is_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should handle empty string values in conversation', () => { + const conversation = { + display_id: '', + }; + const filters = [ + { + attribute_key: 'display_id', + filter_operator: 'is_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Custom attributes tests + it('should match conversation with filter on custom attribute', () => { + const conversation = { + custom_attributes: { + customer_type: 'premium', + }, + }; + const filters = [ + { + attribute_key: 'customer_type', + filter_operator: 'equal_to', + values: 'premium', + query_operator: 'and', + attributeModel: 'customAttributes', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with contains operator on custom attribute', () => { + const conversation = { + custom_attributes: { + notes: 'This customer has requested a refund', + }, + }; + const filters = [ + { + attribute_key: 'notes', + filter_operator: 'contains', + values: 'refund', + query_operator: 'and', + attributeModel: 'customAttributes', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Combination tests + it('should match conversation with combination of different attribute types', () => { + const conversation = { + status: 'open', + created_at: 1647777600000, // March 20, 2022 + custom_attributes: { + customer_type: 'premium', + }, + }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'created_at', + filter_operator: 'is_greater_than', + values: '2022-03-19', // March 19, 2022 + query_operator: 'and', + }, + { + attribute_key: 'customer_type', + filter_operator: 'equal_to', + values: 'premium', + query_operator: 'and', + attributeModel: 'customAttributes', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Backend alignment tests + describe('Backend alignment tests', () => { + // Test case for: status='open' AND priority='urgent' OR display_id='12345' + describe('with A AND B OR C filter chain', () => { + it('matches when all conditions are true', () => { + const conversation = { + status: 'open', // A: true + priority: 'urgent', // B: true + display_id: '12345', // C: true + }; + + // This filter chain is: (status='open' AND priority='urgent') OR display_id='12345' + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '12345', + query_operator: null, + }, + ]; + + // Expected: (true AND true) OR true = true OR true = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('matches when first condition is false but third is true', () => { + const conversation = { + status: 'resolved', // A: false + priority: 'urgent', // B: true + display_id: '12345', // C: true + }; + + // This filter chain is: (status='open' AND priority='urgent') OR display_id='12345' + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '12345', + query_operator: null, + }, + ]; + + // Expected: (false AND true) OR true = false OR true = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('matches when first and second condition is false but third is true', () => { + const conversation = { + status: 'resolved', // A: false + priority: 'low', // B: false + display_id: '12345', // C: true + }; + + // This filter chain is: (status='open' AND priority='urgent') OR display_id='12345' + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '12345', + query_operator: null, + }, + ]; + + // Expected: (false AND false) OR true = false OR true = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('does not match when all conditions are false', () => { + const conversation = { + status: 'resolved', // A: false + priority: 'low', // B: false + display_id: '67890', // C: false + }; + + // This filter chain is: (status='open' AND priority='urgent') OR display_id='12345' + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [ + { + id: 'urgent', + name: 'Urgent', + }, + ], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '12345', + query_operator: null, + }, + ]; + + // Expected: (false AND false) OR false = false OR false = false + expect(matchesFilters(conversation, filters)).toBe(false); + }); + }); + + // Test case for: status='open' OR priority='low' AND display_id='67890' + describe('with A OR B AND C filter chain', () => { + it('matches when first condition is true', () => { + const conversation = { + status: 'open', // A: true + priority: 'urgent', // B: false + display_id: '12345', // C: false + }; + + // This filter chain is: status='open' OR (priority='low' AND display_id='67890') + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'or', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'low', name: 'Low' }], + query_operator: 'and', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '67890', + query_operator: null, + }, + ]; + + // Expected: true OR (false AND false) = true OR false = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('matches when second and third conditions are true', () => { + const conversation = { + status: 'resolved', // A: false + priority: 'low', // B: true + display_id: '67890', // C: true + }; + + // This filter chain is: status='open' OR (priority='low' AND display_id='67890') + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'or', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'low', name: 'Low' }], + query_operator: 'and', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '67890', + query_operator: null, + }, + ]; + + // Expected: false OR (true AND true) = false OR true = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + }); + + // Test case for: status='open' AND priority='urgent' OR display_id='67890' AND browser_language='tr' + describe('with complex filter chain A AND B OR C AND D', () => { + it('matches when first two conditions are true', () => { + const conversation = { + status: 'open', // A: true + priority: 'urgent', // B: true + display_id: '12345', // C: false + additional_attributes: { + browser_language: 'en', // D: false + }, + }; + + // This filter chain is: (status='open' AND priority='urgent') OR (display_id='67890' AND browser_language='tr') + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '67890', + query_operator: 'and', + }, + { + attribute_key: 'browser_language', + filter_operator: 'equal_to', + values: 'tr', + query_operator: null, + }, + ]; + + // Expected: (true AND true) OR (false AND false) = true OR false = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('matches when last two conditions are true', () => { + const conversation = { + status: 'resolved', // A: false + priority: 'low', // B: false + display_id: '67890', // C: true + additional_attributes: { + browser_language: 'tr', // D: true + }, + }; + + // This filter chain is: (status='open' AND priority='urgent') OR (display_id='67890' AND browser_language='tr') + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '67890', + query_operator: 'and', + }, + { + attribute_key: 'browser_language', + filter_operator: 'equal_to', + values: 'tr', + query_operator: null, + }, + ]; + + // Expected: (false AND false) OR (true AND true) = false OR true = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + }); + + // Test case for: status='open' AND (priority='urgent' OR display_id='67890') AND conversation_type='platinum' + describe('with mixed operators filter chain', () => { + it('matches when all conditions in the chain are true', () => { + const conversation = { + status: 'open', // A: true + priority: 'urgent', // B: true + display_id: '12345', // C: false + custom_attributes: { + conversation_type: 'platinum', // D: true + }, + }; + + // This filter chain is: status='open' AND (priority='urgent' OR display_id='67890') AND conversation_type='platinum' + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '67890', + query_operator: 'and', + }, + { + attribute_key: 'conversation_type', + filter_operator: 'equal_to', + values: 'platinum', + query_operator: null, + attributeModel: 'customAttributes', + }, + ]; + + // Expected: true AND (true OR false) AND true = true AND true AND true = true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('does not match when the last condition is false', () => { + const conversation = { + status: 'open', // A: true + priority: 'urgent', // B: true + display_id: '12345', // C: false + custom_attributes: { + conversation_type: 'silver', // D: false + }, + }; + + // This filter chain is: status='open' AND (priority='urgent' OR display_id='67890') AND conversation_type='platinum' + const filters = [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: [{ id: 'open', name: 'Open' }], + query_operator: 'and', + }, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: [{ id: 'urgent', name: 'Urgent' }], + query_operator: 'or', + }, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: '67890', + query_operator: 'and', + }, + { + attribute_key: 'conversation_type', + filter_operator: 'equal_to', + values: 'platinum', + query_operator: null, + attributeModel: 'customAttributes', + }, + ]; + + // true AND true OR false AND false + // true OR false + // true + expect(matchesFilters(conversation, filters)).toBe(true); + }); + }); + }); + + // Test for inbox_id in getValueFromConversation + it('should match conversation with equal_to operator for inbox_id', () => { + const conversation = { inbox_id: 123 }; + const filters = [ + { + attribute_key: 'inbox_id', + filter_operator: 'equal_to', + values: { id: 123, name: 'Support Inbox' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match conversation with equal_to operator for inbox_id when values differ', () => { + const conversation = { inbox_id: 123 }; + const filters = [ + { + attribute_key: 'inbox_id', + filter_operator: 'equal_to', + values: { id: 456, name: 'Sales Inbox' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + // Test for default case (returning null) in getValueFromConversation + it('should not match conversation when attribute key is not recognized', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'unknown_attribute', + filter_operator: 'equal_to', + values: 'some_value', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should match conversation with is_not_present operator for unknown attribute', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'unknown_attribute', + filter_operator: 'is_not_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Test for contains operator when value is not a string + it('should not match conversation with contains operator when value is not a string', () => { + const conversation = { + custom_attributes: { + numeric_value: 12345, + }, + }; + const filters = [ + { + attribute_key: 'numeric_value', + filter_operator: 'contains', + values: '123', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should not match conversation with contains operator when value is an array', () => { + const conversation = { + custom_attributes: { + array_value: [1, 2, 3, 4, 5], + }, + }; + const filters = [ + { + attribute_key: 'array_value', + filter_operator: 'contains', + values: '3', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + // Test for does_not_contain operator when value is not a string + it('should match conversation with does_not_contain operator when value is not a string', () => { + const conversation = { + custom_attributes: { + numeric_value: 12345, + }, + }; + const filters = [ + { + attribute_key: 'numeric_value', + filter_operator: 'does_not_contain', + values: '123', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should match conversation with does_not_contain operator when value is an array', () => { + const conversation = { + custom_attributes: { + array_value: [1, 2, 3, 4, 5], + }, + }; + const filters = [ + { + attribute_key: 'array_value', + filter_operator: 'does_not_contain', + values: '3', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + // Test for default case in matchesCondition + it('should not match conversation with unknown filter operator', () => { + const conversation = { status: 'open' }; + const filters = [ + { + attribute_key: 'status', + filter_operator: 'unknown_operator', + values: 'open', + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + }); +}); diff --git a/package.json b/package.json index adb86f768..9e5bab631 100644 --- a/package.json +++ b/package.json @@ -72,6 +72,7 @@ "highlight.js": "^11.10.0", "idb": "^8.0.0", "js-cookie": "^3.0.5", + "json-logic-js": "^2.0.5", "lettersanitizer": "^1.0.6", "libphonenumber-js": "^1.11.9", "markdown-it": "^13.0.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fdeeac9fb..6749103a8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -136,6 +136,9 @@ importers: js-cookie: specifier: ^3.0.5 version: 3.0.5 + json-logic-js: + specifier: ^2.0.5 + version: 2.0.5 lettersanitizer: specifier: ^1.0.6 version: 1.0.6 @@ -3398,6 +3401,9 @@ packages: json-buffer@3.0.1: resolution: {integrity: sha512-4bV5BfR2mqfQTJm+V5tPPdf+ZpuhiIvTuAB5g8kcrXOZpTT/QwwVRWBywX1ozr6lEuPdbHxwaJlm9G6mI2sfSQ==} + json-logic-js@2.0.5: + resolution: {integrity: sha512-rTT2+lqcuUmj4DgWfmzupZqQDA64AdmYqizzMPWj3DxGdfFNsxPpcNVSaTj4l8W2tG/+hg7/mQhxjU3aPacO6g==} + json-schema-traverse@0.4.1: resolution: {integrity: sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==} @@ -8761,6 +8767,8 @@ snapshots: json-buffer@3.0.1: {} + json-logic-js@2.0.5: {} + json-schema-traverse@0.4.1: {} json-schema-traverse@1.0.0: {} diff --git a/spec/services/conversations/filter_service_spec.rb b/spec/services/conversations/filter_service_spec.rb index 53878c780..5afa92e77 100644 --- a/spec/services/conversations/filter_service_spec.rb +++ b/spec/services/conversations/filter_service_spec.rb @@ -521,4 +521,241 @@ describe Conversations::FilterService do end end end + + describe 'Frontend alignment tests' do + let!(:account) { create(:account) } + let!(:user_1) { create(:user, account: account) } + let!(:inbox) { create(:inbox, account: account) } + let!(:params) { { payload: [], page: 1 } } + + before do + account.conversations.destroy_all + end + + context 'with A AND B OR C filter chain' do + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user_1) } + let(:filter_payload) do + [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: ['open'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: ['urgent'], + query_operator: 'OR' + }.with_indifferent_access, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: ['12345'], + query_operator: nil + }.with_indifferent_access + ] + end + + before do + conversation.update!( + status: 'open', + priority: 'urgent', + display_id: '12345', + additional_attributes: { 'browser_language': 'en' } + ) + end + + it 'matches when all conditions are true' do + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + + it 'matches when first condition is false but third is true' do + conversation.update!(status: 'resolved', priority: 'urgent', display_id: '12345') + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + + it 'matches when first and second condition is false but third is true' do + conversation.update!(status: 'resolved', priority: 'low', display_id: '12345') + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + + it 'does not match when all conditions are false' do + conversation.update!(status: 'resolved', priority: 'low', display_id: '67890') + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 0 + end + end + + context 'with A OR B AND C filter chain' do + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user_1) } + let(:filter_payload) do + [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: ['open'], + query_operator: 'OR' + }.with_indifferent_access, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: ['low'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: ['67890'], + query_operator: nil + }.with_indifferent_access + ] + end + + before do + conversation.update!( + status: 'open', + priority: 'urgent', + display_id: '12345', + additional_attributes: { 'browser_language': 'en' } + ) + end + + it 'matches when first condition is true' do + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + + it 'matches when second and third conditions are true' do + conversation.update!(status: 'resolved', priority: 'low', display_id: '67890') + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + end + + context 'with complex filter chain A AND B OR C AND D' do + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user_1) } + let(:filter_payload) do + [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: ['open'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: ['urgent'], + query_operator: 'OR' + }.with_indifferent_access, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: ['67890'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'browser_language', + filter_operator: 'equal_to', + values: ['tr'], + query_operator: nil + }.with_indifferent_access + ] + end + + before do + conversation.update!( + status: 'open', + priority: 'urgent', + display_id: '12345', + additional_attributes: { 'browser_language': 'en' }, + custom_attributes: { conversation_type: 'platinum' } + ) + end + + it 'matches when first two conditions are true' do + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + + it 'matches when last two conditions are true' do + conversation.update!( + status: 'resolved', + priority: 'low', + display_id: '67890', + additional_attributes: { 'browser_language': 'tr' } + ) + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + end + + context 'with mixed operators filter chain' do + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user_1) } + let(:filter_payload) do + [ + { + attribute_key: 'status', + filter_operator: 'equal_to', + values: ['open'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'priority', + filter_operator: 'equal_to', + values: ['urgent'], + query_operator: 'OR' + }.with_indifferent_access, + { + attribute_key: 'display_id', + filter_operator: 'equal_to', + values: ['67890'], + query_operator: 'AND' + }.with_indifferent_access, + { + attribute_key: 'conversation_type', + filter_operator: 'equal_to', + values: ['platinum'], + custom_attribute_type: '', + query_operator: nil + }.with_indifferent_access + ] + end + + before do + conversation.update!( + status: 'open', + priority: 'urgent', + display_id: '12345', + additional_attributes: { 'browser_language': 'en' }, + custom_attributes: { conversation_type: 'platinum' } + ) + end + + it 'matches when all conditions in the chain are true' do + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + + it 'does not match when the last condition is false' do + conversation.update!(custom_attributes: { conversation_type: 'silver' }) + params[:payload] = filter_payload + result = described_class.new(params, user_1).perform + expect(result[:conversations].length).to be 1 + end + end + end end From 8826a7066c5ed054965de31cf2ce2a9222a80387 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Tue, 25 Mar 2025 09:44:26 +0530 Subject: [PATCH 04/63] chore: Remove delete Instagram story implementation (#11097) Currently, Instagram stories are downloaded and displayed as attachments. We previously implemented story deletion by checking individual messages via https://github.com/chatwoot/chatwoot/pull/5300/files#diff-684a16c1b0b4c099fcdfeed95b1820e11fef629fe332ec7ce6a8c600331dd06dR110, but this approach proved impractical and was removed. This PR removes all unused code to avoid confusion. We will revisit the story deletion feature when we implement `instagram_manage_insights`. --- app/models/channel/facebook_page.rb | 18 ------------ app/models/message.rb | 9 ------ spec/models/channel/facebook_page_spec.rb | 36 ----------------------- 3 files changed, 63 deletions(-) diff --git a/app/models/channel/facebook_page.rb b/app/models/channel/facebook_page.rb index ce5f5d221..1755889f8 100644 --- a/app/models/channel/facebook_page.rb +++ b/app/models/channel/facebook_page.rb @@ -63,22 +63,4 @@ class Channel::FacebookPage < ApplicationRecord Rails.logger.debug { "Rescued: #{e.inspect}" } true end - - # TODO: We will be removing this code after instagram_manage_insights is implemented - def fetch_instagram_story_link(message) - k = Koala::Facebook::API.new(page_access_token) - result = k.get_object(message.source_id, fields: %w[story]) || {} - story_link = result['story']['mention']['link'] - # If the story is expired then it raises the ClientError and if the story is deleted with valid story-id it responses with nil - delete_instagram_story(message) if story_link.blank? - story_link - rescue Koala::Facebook::ClientError => e - Rails.logger.debug { "Instagram Story Expired: #{e.inspect}" } - delete_instagram_story(message) - end - - def delete_instagram_story(message) - message.attachments.destroy_all - message.update(content: I18n.t('conversations.messages.instagram_deleted_story_content'), content_attributes: {}) - end end diff --git a/app/models/message.rb b/app/models/message.rb index 6244d050c..20dad7403 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -155,15 +155,6 @@ class Message < ApplicationRecord } end - # TODO: We will be removing this code after instagram_manage_insights is implemented - # Better logic is to listen to webhook and remove stories proactively rather than trying - # a fetch every time a message is returned - def validate_instagram_story - inbox.channel.fetch_instagram_story_link(self) - # we want to reload the message in case the story has expired and data got removed - reload - end - def merge_sender_attributes(data) data[:sender] = sender.push_event_data if sender && !sender.is_a?(AgentBot) data[:sender] = sender.push_event_data(inbox) if sender.is_a?(AgentBot) diff --git a/spec/models/channel/facebook_page_spec.rb b/spec/models/channel/facebook_page_spec.rb index 5f1374afe..f0f0e036e 100644 --- a/spec/models/channel/facebook_page_spec.rb +++ b/spec/models/channel/facebook_page_spec.rb @@ -30,42 +30,6 @@ RSpec.describe Channel::FacebookPage do channel.prompt_reauthorization! end end - - context 'when fetch instagram story' do - let!(:account) { create(:account) } - let!(:instagram_channel) { create(:channel_instagram_fb_page, account: account, instagram_id: 'chatwoot-app-user-id-1') } - let!(:instagram_inbox) { create(:inbox, channel: instagram_channel, account: account, greeting_enabled: false) } - let(:fb_object) { double } - let(:message) { create(:message, inbox_id: instagram_inbox.id) } - let(:instagram_message) { create(:message, :instagram_story_mention, inbox_id: instagram_inbox.id) } - - it '#fetch_instagram_story_link' do - allow(Koala::Facebook::API).to receive(:new).and_return(fb_object) - allow(fb_object).to receive(:get_object).and_return( - { story: - { - mention: { - link: 'https://www.example.com/test.jpeg', - id: '17920786367196703' - } - }, - from: { - username: 'Sender-id-1', id: 'Sender-id-1' - }, - id: 'instagram-message-id-1234' }.with_indifferent_access - ) - story_link = instagram_channel.fetch_instagram_story_link(message) - expect(story_link).to eq('https://www.example.com/test.jpeg') - end - - it '#delete_instagram_story' do - expect(instagram_message.attachments.count).to eq(1) - - instagram_channel.delete_instagram_story(instagram_message) - - expect(instagram_message.attachments.count).to eq(0) - end - end end it 'has a valid name' do From d797fe34fb4a213db5e269d405fbe139dc783984 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Wed, 26 Mar 2025 08:56:18 +0530 Subject: [PATCH 05/63] fix: Support Business hours when downloading the agent reports. # Pull Request Template ## Description This PR fixes an issue where downloaded agent conversation reports from the Conversations page under Reports do not respect business hours. Fixes https://linear.app/chatwoot/issue/CW-4139/downloaded-agent-reports-do-not-respect-business-hours https://github.com/chatwoot/chatwoot/issues/11057 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ### Loom video https://www.loom.com/share/43b94494647b48c3855476a227b02acb?sid=d6072725-11e5-487c-8aa5-8ecfae6dc818 ## 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 --- .../dashboard/settings/reports/Index.vue | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/app/javascript/dashboard/routes/dashboard/settings/reports/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/reports/Index.vue index 858ceb4ad..0cade8635 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/reports/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/reports/Index.vue @@ -1,11 +1,10 @@ + +