fix: 'room-changed' event race condition

.changeset/odd-squids-repeat.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixes an issue with `room-changed` event not being fired properly when switching between rooms that are available on cache.

apps/meteor/client/hooks/useFireGlobalEvent.spec.ts

@@ -0,0 +1,114 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { renderHook, waitFor } from '@testing-library/react';
+
+import { useFireGlobalEvent } from './useFireGlobalEvent';
+import { fireGlobalEventBase } from '../lib/utils/fireGlobalEventBase';
+
+jest.mock('../lib/utils/fireGlobalEventBase', () => ({
+	fireGlobalEventBase: jest.fn(() => () => undefined),
+}));
+
+const fireGlobalMock = fireGlobalEventBase as jest.MockedFunction<typeof fireGlobalEventBase>;
+
+describe('useFireGlobalEvent', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+	});
+
+	it('should dispatch event only once if scope is defined', async () => {
+		const scope = 'scope';
+		const { result } = renderHook(({ scope }) => useFireGlobalEvent('room-opened', scope), {
+			initialProps: { scope },
+
+			wrapper: mockAppRoot()
+				.withSetting('Iframe_Integration_send_enable', true)
+				.withSetting('Iframe_Integration_send_target_origin', '')
+				.build(),
+		});
+		result.current.mutate(null);
+
+		await waitFor(() => expect(result.current.status).toBe('success'));
+
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+	});
+
+	it('should dispatch event only once for each (eventName/scope)', async () => {
+		const { result, rerender } = renderHook(({ scope }) => useFireGlobalEvent('room-opened', scope), {
+			initialProps: { scope: 'scope' },
+			wrapper: mockAppRoot()
+				.withSetting('Iframe_Integration_send_enable', true)
+				.withSetting('Iframe_Integration_send_target_origin', '')
+				.build(),
+		});
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+
+		rerender({ scope: 'another' });
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(2);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(2);
+	});
+
+	it('should dispatch event multiple times if scope is not defined', async () => {
+		const { result } = renderHook(() => useFireGlobalEvent('room-opened'), {
+			wrapper: mockAppRoot()
+				.withSetting('Iframe_Integration_send_enable', true)
+				.withSetting('Iframe_Integration_send_target_origin', '')
+				.build(),
+		});
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(2);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(3);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(4);
+	});
+
+	it('should pass required settings to postMessage', async () => {
+		const { result } = renderHook(() => useFireGlobalEvent('room-opened'), {
+			wrapper: mockAppRoot()
+				.withSetting('Iframe_Integration_send_enable', true)
+				.withSetting('Iframe_Integration_send_target_origin', 'origin')
+				.build(),
+		});
+
+		const postMessage = jest.fn();
+
+		fireGlobalMock.mockImplementation(() => postMessage);
+
+		result.current.mutate(null);
+		await waitFor(() => expect(result.current.status).toBe('success'));
+		expect(fireGlobalMock).toHaveBeenCalledTimes(1);
+		expect(postMessage).toHaveBeenCalledWith(true, 'origin');
+	});
+});

apps/meteor/client/hooks/useFireGlobalEvent.ts

@@ -0,0 +1,34 @@
+import { useSetting } from '@rocket.chat/ui-contexts';
+import { useMutation } from '@tanstack/react-query';
+import { useEffect, useRef } from 'react';
+
+import { fireGlobalEventBase } from '../lib/utils/fireGlobalEventBase';
+
+const getScopeForEvent = (eventName: string, scope?: string) => (scope ? `${eventName}/${scope}` : eventName);
+
+export const useFireGlobalEvent = (eventName: string, scope?: string) => {
+	const sendEnabled = useSetting('Iframe_Integration_send_enable');
+	const origin = useSetting('Iframe_Integration_send_target_origin');
+
+	const dispatchedRef = useRef({ scope: getScopeForEvent(eventName, scope), dispatched: false });
+
+	useEffect(() => {
+		const newScope = getScopeForEvent(eventName, scope);
+		if (dispatchedRef.current?.scope !== newScope) {
+			dispatchedRef.current = { scope: newScope, dispatched: false };
+		}
+	}, [scope, eventName]);
+
+	return useMutation({
+		mutationFn: async (data?: unknown) => {
+			if (scope && dispatchedRef.current.dispatched) {
+				return;
+			}
+
+			const postMessage = fireGlobalEventBase(eventName, data);
+			postMessage(sendEnabled as boolean, origin as string);
+			dispatchedRef.current.dispatched = true;
+		},
+		scope: scope ? { id: getScopeForEvent(eventName, scope) } : undefined,
+	});
+};

apps/meteor/client/lib/utils/fireGlobalEvent.ts

@@ -1,9 +1,10 @@
 import { Tracker } from 'meteor/tracker';
 
+import { fireGlobalEventBase } from './fireGlobalEventBase';
 import { settings } from '../../../app/settings/client';
 
 export const fireGlobalEvent = (eventName: string, detail?: unknown): void => {
-	window.dispatchEvent(new CustomEvent(eventName, { detail }));
+	const dispatchIframeMessage = fireGlobalEventBase(eventName, detail);
 
 	Tracker.autorun((computation) => {
 		const enabled = settings.get('Iframe_Integration_send_enable');
@@ -13,14 +14,6 @@ export const fireGlobalEvent = (eventName: string, detail?: unknown): void => {
 
 		computation.stop();
 
-		if (enabled) {
-			parent.postMessage(
-				{
-					eventName,
-					data: detail,
-				},
-				settings.get('Iframe_Integration_send_target_origin'),
-			);
-		}
+		dispatchIframeMessage(enabled, settings.get('Iframe_Integration_send_target_origin'));
 	});
 };

apps/meteor/client/lib/utils/fireGlobalEventBase.spec.ts

@@ -0,0 +1,59 @@
+import { fireGlobalEventBase } from './fireGlobalEventBase';
+
+const postMessageMock = jest.fn();
+const dispatchEventMock = jest.fn();
+
+const originalDispatch = window.dispatchEvent;
+const originalPostMessage = parent.postMessage;
+
+beforeAll(() => {
+	window.dispatchEvent = dispatchEventMock;
+	parent.postMessage = postMessageMock;
+});
+
+beforeEach(() => {
+	postMessageMock.mockClear();
+	dispatchEventMock.mockClear();
+});
+
+afterAll(() => {
+	window.dispatchEvent = originalDispatch;
+	parent.postMessage = originalPostMessage;
+});
+
+it('should dispatch event but not post message', () => {
+	const detail = 'test-detail';
+	const postMessage = fireGlobalEventBase('test-event', detail);
+	postMessage(false, '');
+
+	expect(postMessageMock).not.toHaveBeenCalled();
+
+	expect(dispatchEventMock).toHaveBeenCalledTimes(1);
+
+	const result = dispatchEventMock.mock.lastCall[0];
+	expect(result).toBeInstanceOf(CustomEvent);
+	expect(result.detail).toBe(detail);
+	expect(result.type).toBe('test-event');
+});
+
+it('should dispatch event and post message', () => {
+	const detail = 'test-detail';
+	const origin = 'test-origin';
+	const postMessage = fireGlobalEventBase('test-event', detail);
+	postMessage(true, origin);
+
+	expect(postMessageMock).toHaveBeenCalledTimes(1);
+
+	expect(dispatchEventMock).toHaveBeenCalledTimes(1);
+
+	const dispatchResult = dispatchEventMock.mock.lastCall[0];
+	expect(dispatchResult).toBeInstanceOf(CustomEvent);
+	expect(dispatchResult.detail).toBe(detail);
+	expect(dispatchResult.type).toBe('test-event');
+
+	const [postEventResult, originResult] = postMessageMock.mock.lastCall;
+	expect(postEventResult).toBeInstanceOf(Object);
+	expect(originResult).toBe(origin);
+	expect(postEventResult.data).toBe(detail);
+	expect(postEventResult.eventName).toBe('test-event');
+});

apps/meteor/client/lib/utils/fireGlobalEventBase.ts

@@ -0,0 +1,18 @@
+export const fireGlobalEventBase = (eventName: string, detail?: unknown) => {
+	window.dispatchEvent(new CustomEvent(eventName, { detail }));
+
+	const dispatchMessage = (iframeSendEnabled: boolean, sendTargetOrigin: string) => {
+		if (!iframeSendEnabled) {
+			return;
+		}
+		parent.postMessage(
+			{
+				eventName,
+				data: detail,
+			},
+			sendTargetOrigin,
+		);
+	};
+
+	return dispatchMessage;
+};

apps/meteor/client/views/room/hooks/useOpenRoom.ts

@@ -1,12 +1,11 @@
 import type { IRoom, RoomType } from '@rocket.chat/core-typings';
 import { useMethod, usePermission, useRoute, useSetting, useUser } from '@rocket.chat/ui-contexts';
 import { useQuery, useQueryClient } from '@tanstack/react-query';
-import { useEffect, useRef } from 'react';
+import { useEffect } from 'react';
 
 import { useOpenRoomMutation } from './useOpenRoomMutation';
 import { Rooms } from '../../../../app/models/client';
 import { roomFields } from '../../../../lib/publishFields';
-import { omit } from '../../../../lib/utils/omit';
 import { NotAuthorizedError } from '../../../lib/errors/NotAuthorizedError';
 import { NotSubscribedToRoomError } from '../../../lib/errors/NotSubscribedToRoomError';
 import { OldUrlRoomError } from '../../../lib/errors/OldUrlRoomError';
@@ -21,8 +20,6 @@ export function useOpenRoom({ type, reference }: { type: RoomType; reference: st
 	const directRoute = useRoute('direct');
 	const openRoom = useOpenRoomMutation();
 
-	const unsubscribeFromRoomOpenedEvent = useRef<() => void>(() => undefined);
-
 	const result = useQuery({
 		// we need to add uid and username here because `user` is not loaded all at once (see UserProvider -> Meteor.user())
 		queryKey: ['rooms', { reference, type }, { uid: user?._id, username: user?.username }] as const,
@@ -89,10 +86,6 @@ export function useOpenRoom({ type, reference }: { type: RoomType; reference: st
 			}
 
 			const { RoomManager } = await import('../../../lib/RoomManager');
-			const { fireGlobalEvent } = await import('../../../lib/utils/fireGlobalEvent');
-
-			unsubscribeFromRoomOpenedEvent.current();
-			unsubscribeFromRoomOpenedEvent.current = RoomManager.once('opened', () => fireGlobalEvent('room-opened', omit(room, 'usernames')));
 
 			const sub = Subscriptions.findOne({ rid: room._id });
 

apps/meteor/client/views/room/providers/RoomProvider.tsx

@@ -11,6 +11,8 @@ import { useUsersNameChanged } from './hooks/useUsersNameChanged';
 import { Subscriptions } from '../../../../app/models/client';
 import { UserAction } from '../../../../app/ui/client/lib/UserAction';
 import { RoomHistoryManager } from '../../../../app/ui-utils/client';
+import { omit } from '../../../../lib/utils/omit';
+import { useFireGlobalEvent } from '../../../hooks/useFireGlobalEvent';
 import { useReactiveQuery } from '../../../hooks/useReactiveQuery';
 import { useReactiveValue } from '../../../hooks/useReactiveValue';
 import { useRoomInfoEndpoint } from '../../../hooks/useRoomInfoEndpoint';
@@ -86,6 +88,14 @@ const RoomProvider = ({ rid, children }: RoomProviderProps): ReactElement => {
 
 	const isSidepanelFeatureEnabled = useSidePanelNavigation();
 
+	const { mutate: fireRoomOpenedEvent } = useFireGlobalEvent('room-opened', rid);
+
+	useEffect(() => {
+		if (resultFromLocal.data) {
+			fireRoomOpenedEvent(omit(resultFromLocal.data, 'usernames'));
+		}
+	}, [rid, resultFromLocal.data, fireRoomOpenedEvent]);
+
 	useEffect(() => {
 		if (isSidepanelFeatureEnabled) {
 			if (resultFromServer.isSuccess) {