From 84a1e3529488da3ecb1ce271d6f44249c4ca8647 Mon Sep 17 00:00:00 2001 From: Rogee Date: Mon, 24 Aug 2026 00:42:05 +0800 Subject: [PATCH] HH-569: guard replayed frontend realtime messages (#144) * fix(HH-569): guard replayed realtime messages * fix(HH-569): gate dashboard message side effects --------- Co-authored-by: Rogee --- .../dashboard/helper/actionCable.js | 6 +- .../helper/specs/actionCable.spec.js | 156 ++++++++++++++++++ .../store/modules/conversations/actions.js | 34 +++- .../specs/conversations/actions.spec.js | 72 +++++++- .../javascript/widget/helpers/actionCable.js | 22 ++- .../widget/helpers/specs/actionCable.spec.js | 76 +++++++++ .../store/modules/conversation/actions.js | 15 +- .../specs/conversation/actions.spec.js | 32 ++++ 8 files changed, 398 insertions(+), 15 deletions(-) create mode 100644 frontend/app/javascript/widget/helpers/specs/actionCable.spec.js diff --git a/frontend/app/javascript/dashboard/helper/actionCable.js b/frontend/app/javascript/dashboard/helper/actionCable.js index 74b82105..8c1da16c 100644 --- a/frontend/app/javascript/dashboard/helper/actionCable.js +++ b/frontend/app/javascript/dashboard/helper/actionCable.js @@ -100,13 +100,15 @@ class ActionCableConnector extends BaseActionCableConnector { // eslint-disable-next-line class-methods-use-this onLogout = () => AuthAPI.logout(); - onMessageCreated = data => { + onMessageCreated = async data => { const { conversation: { last_activity_at: lastActivityAt }, conversation_id: conversationId, } = data; + const applied = await this.app.$store.dispatch('addMessage', data); + if (!applied) return; + DashboardAudioNotificationHelper.onNewMessage(data); - this.app.$store.dispatch('addMessage', data); this.app.$store.dispatch('updateConversationLastActivity', { lastActivityAt, conversationId, diff --git a/frontend/app/javascript/dashboard/helper/specs/actionCable.spec.js b/frontend/app/javascript/dashboard/helper/specs/actionCable.spec.js index 6c8ad5b8..1ccc7a9f 100644 --- a/frontend/app/javascript/dashboard/helper/specs/actionCable.spec.js +++ b/frontend/app/javascript/dashboard/helper/specs/actionCable.spec.js @@ -1,5 +1,16 @@ import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; import ActionCableConnector from '../actionCable'; +import DashboardAudioNotificationHelper from '../AudioAlerts/DashboardAudioNotificationHelper'; +import conversationActions from '../../store/modules/conversations/actions'; +import { mutations as conversationMutations } from '../../store/modules/conversations'; +import { useCallsStore } from '../../stores/calls'; + +vi.mock('../AudioAlerts/DashboardAudioNotificationHelper', () => ({ + default: { + onNewMessage: vi.fn(), + }, +})); vi.mock('shared/helpers/mitt', () => ({ emitter: { @@ -41,6 +52,27 @@ describe('ActionCableConnector - Copilot Tests', () => { afterEach(() => { vi.useRealTimers(); }); + + const useRealConversationDispatch = state => { + const commit = (type, payload) => + conversationMutations[type](state, payload); + + mockDispatch.mockImplementation((action, payload) => { + if (action === 'addMessage') { + return conversationActions.addMessage( + { commit, state, rootGetters: { getCurrentUserID: 1 } }, + payload + ); + } + if (action === 'updateConversationLastActivity') { + return conversationActions.updateConversationLastActivity( + { commit }, + payload + ); + } + return undefined; + }); + }; describe('copilot event handlers', () => { it('should register the copilot.message.created event handler', () => { expect(Object.keys(actionCable.events)).toContain( @@ -70,6 +102,130 @@ describe('ActionCableConnector - Copilot Tests', () => { }); }); + it('plays new message audio only once when realtime replays a message', async () => { + const message = { + id: 42, + account_id: 1, + conversation_id: 7, + conversation: { last_activity_at: 123 }, + }; + const addMessageResults = [true, false]; + mockDispatch.mockImplementation(action => + Promise.resolve( + action === 'addMessage' ? addMessageResults.shift() : undefined + ) + ); + + await actionCable.onMessageCreated(message); + await actionCable.onMessageCreated(message); + + expect(DashboardAudioNotificationHelper.onNewMessage).toHaveBeenCalledTimes( + 1 + ); + }); + + it('does not apply replayed activity timestamps', async () => { + const currentMessage = { + id: 42, + conversation_id: 7, + updated_at: '2026-08-24T02:00:00Z', + }; + const state = { + allConversations: [ + { + id: 7, + messages: [currentMessage], + last_activity_at: 200, + }, + ], + selectedChatId: null, + }; + useRealConversationDispatch(state); + + await actionCable.onMessageCreated({ + ...currentMessage, + conversation: { last_activity_at: 100 }, + }); + await actionCable.onMessageCreated({ + ...currentMessage, + updated_at: '2026-08-24T01:00:00Z', + conversation: { last_activity_at: 50 }, + }); + + expect(state.allConversations[0].last_activity_at).toBe(200); + expect( + DashboardAudioNotificationHelper.onNewMessage + ).not.toHaveBeenCalled(); + }); + + it('skips dashboard side effects when the conversation mutation is a no-op', async () => { + setActivePinia(createPinia()); + const state = { allConversations: [], selectedChatId: null }; + useRealConversationDispatch(state); + + await actionCable.onMessageCreated({ + id: 42, + conversation_id: 7, + message_type: 1, + content_type: 'voice_call', + updated_at: '2026-08-24T02:00:00Z', + conversation: { last_activity_at: 200 }, + call: { + provider_call_id: 'call-1', + direction: 'incoming', + }, + }); + + expect(state.allConversations).toEqual([]); + expect( + DashboardAudioNotificationHelper.onNewMessage + ).not.toHaveBeenCalled(); + expect(useCallsStore().calls).toEqual([]); + }); + + it('applies a newer event and runs existing dashboard side effects', async () => { + setActivePinia(createPinia()); + const state = { + allConversations: [ + { + id: 7, + messages: [ + { + id: 42, + conversation_id: 7, + updated_at: '2026-08-24T01:00:00Z', + }, + ], + last_activity_at: 100, + }, + ], + selectedChatId: null, + }; + useRealConversationDispatch(state); + const message = { + id: 42, + conversation_id: 7, + message_type: 1, + content: 'newer', + content_type: 'voice_call', + updated_at: '2026-08-24T02:00:00Z', + conversation: { last_activity_at: 200 }, + call: { + provider_call_id: 'call-1', + direction: 'incoming', + }, + }; + + await actionCable.onMessageCreated(message); + + expect(state.allConversations[0].messages[0]).toEqual(message); + expect(state.allConversations[0].last_activity_at).toBe(200); + expect(DashboardAudioNotificationHelper.onNewMessage).toHaveBeenCalledWith( + message + ); + expect(useCallsStore().calls).toHaveLength(1); + }); + describe('conversation unread count event handlers', () => { it('should register the conversation.unread_count_changed event handler', () => { expect(Object.keys(actionCable.events)).toContain( diff --git a/frontend/app/javascript/dashboard/store/modules/conversations/actions.js b/frontend/app/javascript/dashboard/store/modules/conversations/actions.js index 7d3ee463..0b9d5f31 100644 --- a/frontend/app/javascript/dashboard/store/modules/conversations/actions.js +++ b/frontend/app/javascript/dashboard/store/modules/conversations/actions.js @@ -10,6 +10,7 @@ import { isOnUnattendedView, isOnFoldersView, } from './helpers/actionHelpers'; +import { findPendingMessageIndex } from './helpers'; import messageReadActions from './actions/messageReadActions'; import messageTranslateActions from './actions/messageTranslateActions'; import * as Sentry from '@sentry/vue'; @@ -31,6 +32,19 @@ export const hasMessageFailedWithExternalError = pendingMessage => { return status === MESSAGE_STATUS.FAILED && externalError !== ''; }; +const getConversation = (state, message) => + state?.allConversations?.find(item => item.id === message.conversation_id); + +const getStoredMessage = (conversation, message) => { + const index = findPendingMessageIndex(conversation, message); + return conversation.messages[index]; +}; + +const isNewerMessage = (currentMessage, message) => + !currentMessage?.updated_at || + !message.updated_at || + message.updated_at > currentMessage.updated_at; + // actions const actions = { getConversation: async ({ commit }, conversationId) => { @@ -342,7 +356,13 @@ const actions = { } }, - addMessage({ commit, rootGetters }, message) { + addMessage({ commit, state, rootGetters }, message) { + const conversation = getConversation(state, message); + if (!conversation) return false; + + const currentMessage = getStoredMessage(conversation, message); + if (!isNewerMessage(currentMessage, message)) return false; + commit(types.ADD_MESSAGE, message); if (message.message_type === MESSAGE_TYPE.INCOMING) { commit(types.SET_CONVERSATION_CAN_REPLY, { @@ -352,11 +372,21 @@ const actions = { commit(types.ADD_CONVERSATION_ATTACHMENTS, message); } handleVoiceCallCreated(message, rootGetters?.getCurrentUserID); + return true; }, - updateMessage({ commit, rootGetters }, message) { + updateMessage({ commit, state, rootGetters }, message) { + const conversation = getConversation(state, message); + if ( + !conversation || + !isNewerMessage(getStoredMessage(conversation, message), message) + ) { + return false; + } + commit(types.ADD_MESSAGE, message); handleVoiceCallUpdated(commit, message, rootGetters?.getCurrentUserID); + return true; }, deleteMessage: async function deleteLabels( diff --git a/frontend/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js b/frontend/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js index 8a4f3d56..d5ea98b5 100644 --- a/frontend/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js +++ b/frontend/app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js @@ -278,7 +278,11 @@ describe('#actions', () => { message_type: 0, conversation_id: 1, }; - actions.addMessage({ commit }, message); + const state = { + allConversations: [{ id: 1, messages: [] }], + }; + const applied = actions.addMessage({ commit, state }, message); + expect(applied).toBe(true); expect(commit.mock.calls).toEqual([ [types.ADD_MESSAGE, message], [ @@ -294,9 +298,73 @@ describe('#actions', () => { message_type: 1, conversation_id: 1, }; - actions.addMessage({ commit }, message); + const state = { + allConversations: [{ id: 1, messages: [] }], + }; + actions.addMessage({ commit, state }, message); expect(commit.mock.calls).toEqual([[types.ADD_MESSAGE, message]]); }); + + it('returns false if the conversation is not loaded', () => { + const message = { + id: 1, + message_type: 1, + conversation_id: 1, + }; + + const applied = actions.addMessage( + { commit, state: { allConversations: [] } }, + message + ); + + expect(applied).toBe(false); + expect(commit).not.toHaveBeenCalled(); + }); + + it('ignores a replayed message with the same id and version', () => { + const message = { + id: 1, + message_type: 1, + conversation_id: 1, + updated_at: '2026-08-24T02:00:00Z', + }; + const state = { + allConversations: [{ id: 1, messages: [message] }], + }; + + const isNew = actions.addMessage({ commit, state }, message); + + expect(isNew).toBe(false); + expect(commit).not.toHaveBeenCalled(); + }); + }); + + describe('#updateMessage', () => { + it('ignores an older realtime update', () => { + const message = { + id: 1, + conversation_id: 1, + updated_at: '2026-08-24T01:00:00Z', + }; + const state = { + allConversations: [ + { + id: 1, + messages: [ + { + id: 1, + updated_at: '2026-08-24T02:00:00Z', + }, + ], + }, + ], + }; + + const applied = actions.updateMessage({ commit, state }, message); + + expect(applied).toBe(false); + expect(commit).not.toHaveBeenCalled(); + }); }); describe('#sendMessageWithData', () => { diff --git a/frontend/app/javascript/widget/helpers/actionCable.js b/frontend/app/javascript/widget/helpers/actionCable.js index 60c379ed..298337c3 100644 --- a/frontend/app/javascript/widget/helpers/actionCable.js +++ b/frontend/app/javascript/widget/helpers/actionCable.js @@ -53,14 +53,18 @@ class ActionCableConnector extends BaseActionCableConnector { this.app.$store.dispatch('conversationAttributes/update', data); }; - onMessageCreated = data => { + onMessageCreated = async data => { if (isMessageInActiveConversation(this.app.$store.getters, data)) { return; } - this.app.$store - .dispatch('conversation/addOrUpdateMessage', data) - .then(() => emitter.emit(ON_AGENT_MESSAGE_RECEIVED)); + const { applied, isNew } = await this.app.$store.dispatch( + 'conversation/addOrUpdateMessage', + data + ); + if (!applied || !isNew) return; + + emitter.emit(ON_AGENT_MESSAGE_RECEIVED); IFrameHelper.sendMessage({ event: 'onEvent', @@ -72,11 +76,17 @@ class ActionCableConnector extends BaseActionCableConnector { } }; - onMessageUpdated = data => { + onMessageUpdated = async data => { if (isMessageInActiveConversation(this.app.$store.getters, data)) { return; } + const { applied } = await this.app.$store.dispatch( + 'conversation/addOrUpdateMessage', + data + ); + if (!applied) return; + if (shouldTriggerMessageUpdateEvent(data)) { IFrameHelper.sendMessage({ event: 'onEvent', @@ -84,8 +94,6 @@ class ActionCableConnector extends BaseActionCableConnector { data, }); } - - this.app.$store.dispatch('conversation/addOrUpdateMessage', data); }; onConversationCreated = () => { diff --git a/frontend/app/javascript/widget/helpers/specs/actionCable.spec.js b/frontend/app/javascript/widget/helpers/specs/actionCable.spec.js new file mode 100644 index 00000000..85f865bf --- /dev/null +++ b/frontend/app/javascript/widget/helpers/specs/actionCable.spec.js @@ -0,0 +1,76 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import ActionCableConnector from '../actionCable'; +import { playNewMessageNotificationInWidget } from '../WidgetAudioNotificationHelper'; +import { IFrameHelper } from '../utils'; +import { emitter } from 'shared/helpers/mitt'; + +vi.mock('../WidgetAudioNotificationHelper', () => ({ + playNewMessageNotificationInWidget: vi.fn(), +})); + +vi.mock('../utils', () => ({ + IFrameHelper: { + sendMessage: vi.fn(), + }, +})); + +vi.mock('shared/helpers/mitt', () => ({ + emitter: { + emit: vi.fn(), + }, +})); + +describe('Widget ActionCableConnector', () => { + let actionCable; + let dispatch; + + beforeEach(() => { + dispatch = vi.fn(); + actionCable = new ActionCableConnector( + { + $store: { + dispatch, + getters: { + getCurrentAccountId: 1, + getCurrentUserID: 1, + 'conversationAttributes/getConversationParams': { id: 7 }, + }, + }, + }, + 'test-token' + ); + }); + + it('emits message side effects only once when realtime replays a message', async () => { + const message = { + id: 42, + conversation_id: 7, + sender_type: 'User', + }; + dispatch + .mockResolvedValueOnce({ applied: true, isNew: true }) + .mockResolvedValueOnce({ applied: false, isNew: false }); + + await actionCable.onMessageCreated(message); + await actionCable.onMessageCreated(message); + + expect(emitter.emit).toHaveBeenCalledTimes(1); + expect(IFrameHelper.sendMessage).toHaveBeenCalledTimes(1); + expect(playNewMessageNotificationInWidget).toHaveBeenCalledTimes(1); + }); + + it('does not emit the SDK callback for an older message update', async () => { + const message = { + id: 42, + conversation_id: 7, + previous_changes: { + content_attributes: [{}, { submitted_values: ['yes'] }], + }, + }; + dispatch.mockResolvedValue({ applied: false, isNew: false }); + + await actionCable.onMessageUpdated(message); + + expect(IFrameHelper.sendMessage).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/app/javascript/widget/store/modules/conversation/actions.js b/frontend/app/javascript/widget/store/modules/conversation/actions.js index 1e2b06c7..30e0cf93 100644 --- a/frontend/app/javascript/widget/store/modules/conversation/actions.js +++ b/frontend/app/javascript/widget/store/modules/conversation/actions.js @@ -173,13 +173,24 @@ export const actions = { commit('clearConversations'); }, - addOrUpdateMessage: async ({ commit }, data) => { + addOrUpdateMessage: async ({ commit, state }, data) => { const { id, content_attributes } = data; + const currentMessage = state?.conversations?.[id]; + const isNew = !currentMessage; + if ( + currentMessage?.updated_at && + data.updated_at && + data.updated_at <= currentMessage.updated_at + ) { + return { applied: false, isNew }; + } + if (content_attributes && content_attributes.deleted) { commit('deleteMessage', id); - return; + return { applied: true, isNew }; } commit('pushMessageToConversation', data); + return { applied: true, isNew }; }, toggleAgentTyping({ commit }, data) { diff --git a/frontend/app/javascript/widget/store/modules/specs/conversation/actions.spec.js b/frontend/app/javascript/widget/store/modules/specs/conversation/actions.spec.js index 811e08d9..7f0c6215 100644 --- a/frontend/app/javascript/widget/store/modules/specs/conversation/actions.spec.js +++ b/frontend/app/javascript/widget/store/modules/specs/conversation/actions.spec.js @@ -82,6 +82,38 @@ describe('#actions', () => { message_type: 1, }); }); + + it('ignores an older realtime update', async () => { + const data = { + id: 1, + updated_at: '2026-08-24T01:00:00Z', + content_attributes: {}, + }; + const state = { + conversations: { + 1: { id: 1, updated_at: '2026-08-24T02:00:00Z' }, + }, + }; + + const result = await actions.addOrUpdateMessage({ commit, state }, data); + + expect(result).toEqual({ applied: false, isNew: false }); + expect(commit).not.toHaveBeenCalled(); + }); + + it('ignores a replayed message with the same id and version', async () => { + const data = { + id: 1, + updated_at: '2026-08-24T02:00:00Z', + content_attributes: {}, + }; + const state = { conversations: { 1: data } }; + + const result = await actions.addOrUpdateMessage({ commit, state }, data); + + expect(result).toEqual({ applied: false, isNew: false }); + expect(commit).not.toHaveBeenCalled(); + }); }); describe('#toggleAgentTyping', () => {