From d902bb1d6f890e8b8edcf465171721a9bd803c48 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Sat, 7 Dec 2024 02:01:01 +0530 Subject: [PATCH] fix: Remove duplicate contactable inbox in the conversation form (#10554) --------- Co-authored-by: Pranav --- .../api/v1/accounts/contacts_controller.rb | 2 +- app/javascript/dashboard/api/contacts.js | 8 +++ .../Contacts/Pages/ContactDetails.vue | 4 ++ .../NewConversation/ComposeConversation.vue | 1 - .../helpers/composeConversationHelper.js | 69 +++++++++++++----- .../specs/composeConversationHelper.spec.js | 71 +++++++++++++++++++ .../contacts/pages/ContactManageView.vue | 2 +- .../v1/accounts/contacts/show.json.jbuilder | 2 +- .../v1/accounts/contacts/update.json.jbuilder | 2 +- 9 files changed, 137 insertions(+), 24 deletions(-) diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb index 250c64a86..0d8e0ed93 100644 --- a/app/controllers/api/v1/accounts/contacts_controller.rb +++ b/app/controllers/api/v1/accounts/contacts_controller.rb @@ -14,7 +14,7 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController before_action :check_authorization before_action :set_current_page, only: [:index, :active, :search, :filter] before_action :fetch_contact, only: [:show, :update, :destroy, :avatar, :contactable_inboxes, :destroy_custom_attributes] - before_action :set_include_contact_inboxes, only: [:index, :search, :filter] + before_action :set_include_contact_inboxes, only: [:index, :search, :filter, :show, :update] def index @contacts_count = resolved_contacts.count diff --git a/app/javascript/dashboard/api/contacts.js b/app/javascript/dashboard/api/contacts.js index 85c4bba1d..2eee3f484 100644 --- a/app/javascript/dashboard/api/contacts.js +++ b/app/javascript/dashboard/api/contacts.js @@ -27,6 +27,14 @@ class ContactAPI extends ApiClient { return axios.get(requestURL); } + show(id) { + return axios.get(`${this.url}/${id}?include_contact_inboxes=false`); + } + + update(id, data) { + return axios.patch(`${this.url}/${id}?include_contact_inboxes=false`, data); + } + getConversations(contactId) { return axios.get(`${this.url}/${contactId}/conversations`); } diff --git a/app/javascript/dashboard/components-next/Contacts/Pages/ContactDetails.vue b/app/javascript/dashboard/components-next/Contacts/Pages/ContactDetails.vue index d46ffd850..aec21a976 100644 --- a/app/javascript/dashboard/components-next/Contacts/Pages/ContactDetails.vue +++ b/app/javascript/dashboard/components-next/Contacts/Pages/ContactDetails.vue @@ -70,6 +70,10 @@ const updateContact = async () => { try { const { customAttributes, ...basicContactData } = contactData.value; await store.dispatch('contacts/update', basicContactData); + await store.dispatch( + 'contacts/fetchContactableInbox', + props.selectedContact.id + ); useAlert(t('CONTACTS_LAYOUT.CARD.EDIT_DETAILS_FORM.SUCCESS_MESSAGE')); } catch (error) { useAlert(t('CONTACTS_LAYOUT.CARD.EDIT_DETAILS_FORM.ERROR_MESSAGE')); diff --git a/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue b/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue index 2ec22140b..b5d57f202 100644 --- a/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue +++ b/app/javascript/dashboard/components-next/NewConversation/ComposeConversation.vue @@ -153,7 +153,6 @@ watch( activeContact, () => { if (activeContact.value && props.contactId) { - // Add null check for contactInboxes const contactInboxes = activeContact.value?.contactInboxes || []; selectedContact.value = { ...activeContact.value, diff --git a/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js b/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js index c6bb9b526..12a5e76b3 100644 --- a/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js +++ b/app/javascript/dashboard/components-next/NewConversation/helpers/composeConversationHelper.js @@ -3,6 +3,15 @@ import { getInboxIconByType } from 'dashboard/helper/inbox'; import camelcaseKeys from 'camelcase-keys'; import ContactAPI from 'dashboard/api/contacts'; +const CHANNEL_PRIORITY = { + 'Channel::Email': 1, + 'Channel::Whatsapp': 2, + 'Channel::Sms': 3, + 'Channel::TwilioSms': 4, + 'Channel::WebWidget': 5, + 'Channel::Api': 6, +}; + export const generateLabelForContactableInboxesList = ({ name, email, @@ -21,27 +30,49 @@ export const generateLabelForContactableInboxesList = ({ return name; }; +const transformInbox = ({ + name, + id, + email, + channelType, + phoneNumber, + ...rest +}) => ({ + id, + icon: getInboxIconByType(channelType, phoneNumber, 'line'), + label: generateLabelForContactableInboxesList({ + name, + email, + channelType, + phoneNumber, + }), + action: 'inbox', + value: id, + name, + email, + phoneNumber, + channelType, + ...rest, +}); + +export const compareInboxes = (a, b) => { + // Channels that have no priority defined should come at the end. + const priorityA = CHANNEL_PRIORITY[a.channelType] || 999; + const priorityB = CHANNEL_PRIORITY[b.channelType] || 999; + + if (priorityA !== priorityB) { + return priorityA - priorityB; + } + + const nameA = a.name || ''; + const nameB = b.name || ''; + return nameA.localeCompare(nameB); +}; + export const buildContactableInboxesList = contactInboxes => { if (!contactInboxes) return []; - return contactInboxes.map( - ({ name, id, email, channelType, phoneNumber, ...rest }) => ({ - id, - icon: getInboxIconByType(channelType, phoneNumber, 'line'), - label: generateLabelForContactableInboxesList({ - name, - email, - channelType, - phoneNumber, - }), - action: 'inbox', - value: id, - name, - email, - phoneNumber, - channelType, - ...rest, - }) - ); + + return contactInboxes.map(transformInbox).sort(compareInboxes); }; export const getCapitalizedNameFromEmail = email => { 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 54c4eab60..5a2d0092d 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 @@ -463,3 +463,74 @@ describe('composeConversationHelper', () => { }); }); }); + +describe('compareInboxes', () => { + it('should sort inboxes by channel priority', () => { + const inboxes = [ + { channelType: 'Channel::Api', name: 'API Inbox' }, + { channelType: 'Channel::Email', name: 'Email Inbox' }, + { channelType: 'Channel::WebWidget', name: 'Widget' }, + { channelType: 'Channel::Whatsapp', name: 'WhatsApp' }, + ]; + + const sorted = [...inboxes].sort(helpers.compareInboxes); + + expect(sorted[0].channelType).toBe('Channel::Email'); + expect(sorted[1].channelType).toBe('Channel::Whatsapp'); + expect(sorted[2].channelType).toBe('Channel::WebWidget'); + expect(sorted[3].channelType).toBe('Channel::Api'); + }); + + it('should sort SMS channels correctly', () => { + const inboxes = [ + { channelType: 'Channel::TwilioSms', name: 'Twilio' }, + { channelType: 'Channel::Sms', name: 'Regular SMS' }, + ]; + + const sorted = [...inboxes].sort(helpers.compareInboxes); + + expect(sorted[0].channelType).toBe('Channel::Sms'); + expect(sorted[1].channelType).toBe('Channel::TwilioSms'); + }); + + it('should sort by name when channel types are same', () => { + const inboxes = [ + { channelType: 'Channel::Email', name: 'Support' }, + { channelType: 'Channel::Email', name: 'Marketing' }, + { channelType: 'Channel::Email', name: 'Billing' }, + ]; + + const sorted = [...inboxes].sort(helpers.compareInboxes); + + expect(sorted.map(inbox => inbox.name)).toEqual([ + 'Billing', + 'Marketing', + 'Support', + ]); + }); + + it('should put channels without priority at the end', () => { + const inboxes = [ + { channelType: 'Channel::Unknown', name: 'Unknown' }, + { channelType: 'Channel::Email', name: 'Email' }, + { channelType: 'Channel::LineChannel', name: 'Line' }, + { channelType: 'Channel::Whatsapp', name: 'WhatsApp' }, + ]; + + const sorted = [...inboxes].sort(helpers.compareInboxes); + + expect(sorted.map(i => i.channelType)).toEqual([ + 'Channel::Email', + 'Channel::Whatsapp', + + 'Channel::LineChannel', + 'Channel::Unknown', + ]); + }); + + it('should handle empty array', () => { + const inboxes = []; + const sorted = [...inboxes].sort(helpers.compareInboxes); + expect(sorted).toEqual([]); + }); +}); diff --git a/app/javascript/dashboard/routes/dashboard/contacts/pages/ContactManageView.vue b/app/javascript/dashboard/routes/dashboard/contacts/pages/ContactManageView.vue index 8b07646e0..9f99157e6 100644 --- a/app/javascript/dashboard/routes/dashboard/contacts/pages/ContactManageView.vue +++ b/app/javascript/dashboard/routes/dashboard/contacts/pages/ContactManageView.vue @@ -62,7 +62,7 @@ const goToContactsList = () => { const fetchActiveContact = async () => { if (route.params.contactId) { - store.dispatch('contacts/show', { id: route.params.contactId }); + await store.dispatch('contacts/show', { id: route.params.contactId }); await store.dispatch( 'contacts/fetchContactableInbox', route.params.contactId diff --git a/app/views/api/v1/accounts/contacts/show.json.jbuilder b/app/views/api/v1/accounts/contacts/show.json.jbuilder index 524a393bf..1add73326 100644 --- a/app/views/api/v1/accounts/contacts/show.json.jbuilder +++ b/app/views/api/v1/accounts/contacts/show.json.jbuilder @@ -1,3 +1,3 @@ json.payload do - json.partial! 'api/v1/models/contact', formats: [:json], resource: @contact, with_contact_inboxes: true + json.partial! 'api/v1/models/contact', formats: [:json], resource: @contact, with_contact_inboxes: @include_contact_inboxes end diff --git a/app/views/api/v1/accounts/contacts/update.json.jbuilder b/app/views/api/v1/accounts/contacts/update.json.jbuilder index 524a393bf..1add73326 100644 --- a/app/views/api/v1/accounts/contacts/update.json.jbuilder +++ b/app/views/api/v1/accounts/contacts/update.json.jbuilder @@ -1,3 +1,3 @@ json.payload do - json.partial! 'api/v1/models/contact', formats: [:json], resource: @contact, with_contact_inboxes: true + json.partial! 'api/v1/models/contact', formats: [:json], resource: @contact, with_contact_inboxes: @include_contact_inboxes end