From d3d39a81d6614d016663a710338d177063fc49de Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Thu, 23 Jan 2025 12:53:28 +0530 Subject: [PATCH] fix: Prevent duplicate chat creation in the web widget during latency (#10745) --- app/javascript/shared/components/Button.vue | 3 +- .../widget/components/PreChat/Form.vue | 8 +++-- app/javascript/widget/router.js | 35 ++++++++++++++++++- .../widget/store/modules/appConfig.js | 13 +++++++ .../modules/specs/appConfig/actions.spec.js | 7 ++++ .../modules/specs/appConfig/getters.spec.js | 6 ++++ .../modules/specs/appConfig/mutations.spec.js | 8 +++++ .../specs/conversation/actions.spec.js | 1 + app/javascript/widget/store/types.js | 1 + app/javascript/widget/views/PreChatForm.vue | 16 ++++++--- 10 files changed, 90 insertions(+), 8 deletions(-) diff --git a/app/javascript/shared/components/Button.vue b/app/javascript/shared/components/Button.vue index e49c380e0..5d4213d0b 100644 --- a/app/javascript/shared/components/Button.vue +++ b/app/javascript/shared/components/Button.vue @@ -24,7 +24,8 @@ export default { }, computed: { buttonClassName() { - let className = 'text-white py-3 px-4 rounded shadow-sm leading-4'; + let className = + 'text-white py-3 px-4 rounded shadow-sm leading-4 cursor-pointer disabled:opacity-50'; if (this.type === 'clear') { className = 'flex mx-auto mt-4 text-xs leading-3 w-auto text-black-600'; } diff --git a/app/javascript/widget/components/PreChat/Form.vue b/app/javascript/widget/components/PreChat/Form.vue index 6b2c93cd6..280cfbf43 100644 --- a/app/javascript/widget/components/PreChat/Form.vue +++ b/app/javascript/widget/components/PreChat/Form.vue @@ -52,9 +52,13 @@ export default { ...mapGetters({ widgetColor: 'appConfig/getWidgetColor', isCreating: 'conversation/getIsCreating', + isConversationRouting: 'appConfig/getIsUpdatingRoute', activeCampaign: 'campaign/getActiveCampaign', currentUser: 'contacts/getCurrentUser', }), + isCreatingConversation() { + return this.isCreating || this.isConversationRouting; + }, textColor() { return getContrastingTextColor(this.widgetColor); }, @@ -337,9 +341,9 @@ export default { block :bg-color="widgetColor" :text-color="textColor" - :disabled="isCreating" + :disabled="isCreatingConversation" > - + {{ $t('START_CONVERSATION') }} diff --git a/app/javascript/widget/router.js b/app/javascript/widget/router.js index 418fd9ae6..e8230c2d9 100755 --- a/app/javascript/widget/router.js +++ b/app/javascript/widget/router.js @@ -1,7 +1,8 @@ import { createRouter, createWebHashHistory } from 'vue-router'; import ViewWithHeader from './components/layouts/ViewWithHeader.vue'; +import store from './store'; -export default createRouter({ +const router = createRouter({ history: createWebHashHistory(), routes: [ { @@ -42,3 +43,35 @@ export default createRouter({ }, ], }); + +/** + * Navigation Guards to Handle Route Transitions + * + * Purpose: + * Prevents duplicate form submissions and API calls during route transitions, + * especially important in high-latency scenarios. + * + * Flow: + * 1. beforeEach: Sets isUpdatingRoute to true at start of navigation + * 2. Component buttons/actions check this flag to prevent duplicate actions + * 3. afterEach: Resets the flag once navigation is complete + * + * Implementation note: + * Handling it globally, so that we can use it across all components + * to ensure consistent UI behavior during all route transitions. + * + * @see https://github.com/chatwoot/chatwoot/issues/10736 + */ + +router.beforeEach(async (to, from, next) => { + // Prevent any user interactions during route transition + await store.dispatch('appConfig/setRouteTransitionState', true); + next(); +}); + +router.afterEach(() => { + // Re-enable user interactions after navigation is complete + store.dispatch('appConfig/setRouteTransitionState', false); +}); + +export default router; diff --git a/app/javascript/widget/store/modules/appConfig.js b/app/javascript/widget/store/modules/appConfig.js index a7df60d29..44a13c4c4 100644 --- a/app/javascript/widget/store/modules/appConfig.js +++ b/app/javascript/widget/store/modules/appConfig.js @@ -5,6 +5,7 @@ import { SET_WIDGET_APP_CONFIG, SET_WIDGET_COLOR, TOGGLE_WIDGET_OPEN, + SET_ROUTE_UPDATE_STATE, } from '../types'; const state = { @@ -19,6 +20,7 @@ const state = { widgetColor: '', widgetStyle: 'standard', darkMode: 'light', + isUpdatingRoute: false, }; export const getters = { @@ -31,6 +33,7 @@ export const getters = { isWidgetStyleFlat: $state => $state.widgetStyle === 'flat', darkMode: $state => $state.darkMode, getShowUnreadMessagesDialog: $state => $state.showUnreadMessagesDialog, + getIsUpdatingRoute: _state => _state.isUpdatingRoute, }; export const actions = { @@ -69,6 +72,13 @@ export const actions = { setBubbleVisibility({ commit }, hideMessageBubble) { commit(SET_BUBBLE_VISIBILITY, hideMessageBubble); }, + setRouteTransitionState: async ({ commit }, status) => { + // Handles the routing state during navigation to different screen + // Called before the navigation starts and after navigation completes + // Handling this state in app/javascript/widget/router.js + // See issue: https://github.com/chatwoot/chatwoot/issues/10736 + commit(SET_ROUTE_UPDATE_STATE, status); + }, }; export const mutations = { @@ -96,6 +106,9 @@ export const mutations = { [SET_COLOR_SCHEME]($state, darkMode) { $state.darkMode = darkMode; }, + [SET_ROUTE_UPDATE_STATE]($state, status) { + $state.isUpdatingRoute = status; + }, }; export default { diff --git a/app/javascript/widget/store/modules/specs/appConfig/actions.spec.js b/app/javascript/widget/store/modules/specs/appConfig/actions.spec.js index 4c66600f2..a99079562 100644 --- a/app/javascript/widget/store/modules/specs/appConfig/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/appConfig/actions.spec.js @@ -31,4 +31,11 @@ describe('#actions', () => { expect(commit.mock.calls).toEqual([['SET_COLOR_SCHEME', 'dark']]); }); }); + + describe('#setRouteTransitionState', () => { + it('creates actions properly', () => { + actions.setRouteTransitionState({ commit }, false); + expect(commit.mock.calls).toEqual([['SET_ROUTE_UPDATE_STATE', false]]); + }); + }); }); diff --git a/app/javascript/widget/store/modules/specs/appConfig/getters.spec.js b/app/javascript/widget/store/modules/specs/appConfig/getters.spec.js index ab0c6e6ca..5d3db77bc 100644 --- a/app/javascript/widget/store/modules/specs/appConfig/getters.spec.js +++ b/app/javascript/widget/store/modules/specs/appConfig/getters.spec.js @@ -19,4 +19,10 @@ describe('#getters', () => { expect(getters.getShowUnreadMessagesDialog(state)).toEqual(true); }); }); + describe('#getIsUpdatingRoute', () => { + it('returns correct value', () => { + const state = { isUpdatingRoute: true }; + expect(getters.getIsUpdatingRoute(state)).toEqual(true); + }); + }); }); diff --git a/app/javascript/widget/store/modules/specs/appConfig/mutations.spec.js b/app/javascript/widget/store/modules/specs/appConfig/mutations.spec.js index e79235e28..e25fea368 100644 --- a/app/javascript/widget/store/modules/specs/appConfig/mutations.spec.js +++ b/app/javascript/widget/store/modules/specs/appConfig/mutations.spec.js @@ -32,4 +32,12 @@ describe('#mutations', () => { expect(state.darkMode).toEqual('dark'); }); }); + + describe('#SET_ROUTE_UPDATE_STATE', () => { + it('sets dark mode properly', () => { + const state = { isUpdatingRoute: false }; + mutations.SET_ROUTE_UPDATE_STATE(state, true); + expect(state.isUpdatingRoute).toEqual(true); + }); + }); }); diff --git a/app/javascript/widget/store/modules/specs/conversation/actions.spec.js b/app/javascript/widget/store/modules/specs/conversation/actions.spec.js index dbf6f692b..39b8afe1a 100644 --- a/app/javascript/widget/store/modules/specs/conversation/actions.spec.js +++ b/app/javascript/widget/store/modules/specs/conversation/actions.spec.js @@ -17,6 +17,7 @@ describe('#actions', () => { messages: [{ id: 1, content: 'This is a test message' }], }, }); + let windowSpy = vi.spyOn(window, 'window', 'get'); windowSpy.mockImplementation(() => ({ WOOT_WIDGET: { diff --git a/app/javascript/widget/store/types.js b/app/javascript/widget/store/types.js index 17398b682..b4c2a968f 100644 --- a/app/javascript/widget/store/types.js +++ b/app/javascript/widget/store/types.js @@ -7,3 +7,4 @@ export const UPDATE_CONVERSATION_ATTRIBUTES = 'UPDATE_CONVERSATION_ATTRIBUTES'; export const TOGGLE_WIDGET_OPEN = 'TOGGLE_WIDGET_OPEN'; export const SET_REFERRER_HOST = 'SET_REFERRER_HOST'; export const SET_BUBBLE_VISIBILITY = 'SET_BUBBLE_VISIBILITY'; +export const SET_ROUTE_UPDATE_STATE = 'SET_ROUTE_UPDATE_STATE'; diff --git a/app/javascript/widget/views/PreChatForm.vue b/app/javascript/widget/views/PreChatForm.vue index d47e03736..8eac55808 100644 --- a/app/javascript/widget/views/PreChatForm.vue +++ b/app/javascript/widget/views/PreChatForm.vue @@ -12,12 +12,20 @@ export default { }, mixins: [configMixin, routerMixin], mounted() { - emitter.on(ON_CONVERSATION_CREATED, () => { - // Redirect to messages page after conversation is created - this.replaceRoute('messages'); - }); + // Register event listener for conversation creation + emitter.on(ON_CONVERSATION_CREATED, this.handleConversationCreated); + }, + beforeUnmount() { + emitter.off(ON_CONVERSATION_CREATED, this.handleConversationCreated); }, methods: { + handleConversationCreated() { + // Redirect to messages page after conversation is created + this.replaceRoute('messages'); + // Only after successful navigation, reset the isUpdatingRoute UIflag in app/javascript/widget/router.js + // See issue: https://github.com/chatwoot/chatwoot/issues/10736 + }, + onSubmit({ fullName, emailAddress,