fix: Surface elements (Modals, Contextual Bars) fail to respond when triggered by app submit events (#32073)

Co-authored-by: Tasso Evangelista <2263066+tassoevan@users.noreply.github.com>

.changeset/young-candles-explode.md

@@ -0,0 +1,6 @@
+---
+'@rocket.chat/ui-contexts': minor
+'@rocket.chat/meteor': minor
+---
+
+Fixed an issue affecting the update modal/contextual bar by apps when it comes to error handling and regular surface update

apps/meteor/app/ui-message/client/ActionManager.ts

@@ -3,11 +3,14 @@ import { Emitter } from '@rocket.chat/emitter';
 import { Random } from '@rocket.chat/random';
 import type { RouterContext, IActionManager } from '@rocket.chat/ui-contexts';
 import type * as UiKit from '@rocket.chat/ui-kit';
+import { t } from 'i18next';
 import type { ContextType } from 'react';
 import { lazy } from 'react';
 
 import * as banners from '../../../client/lib/banners';
 import { imperativeModal } from '../../../client/lib/imperativeModal';
+import { dispatchToastMessage } from '../../../client/lib/toast';
+import { exhaustiveCheck } from '../../../lib/utils/exhaustiveCheck';
 import { sdk } from '../../utils/client/lib/SDKClient';
 import { UiKitTriggerTimeoutError } from './UiKitTriggerTimeoutError';
 
@@ -20,7 +23,7 @@ export class ActionManager implements IActionManager {
 
 	protected events = new Emitter<{ busy: { busy: boolean }; [viewId: string]: any }>();
 
-	protected triggersId = new Map<string, string | undefined>();
+	protected appIdByTriggerId = new Map<string, string | undefined>();
 
 	protected viewInstances = new Map<
 		string,
@@ -35,8 +38,8 @@ export class ActionManager implements IActionManager {
 	public constructor(protected router: ContextType<typeof RouterContext>) {}
 
 	protected invalidateTriggerId(id: string) {
-		const appId = this.triggersId.get(id);
-		this.triggersId.delete(id);
+		const appId = this.appIdByTriggerId.get(id);
+		this.appIdByTriggerId.delete(id);
 		return appId;
 	}
 
@@ -66,41 +69,75 @@ export class ActionManager implements IActionManager {
 
 	public generateTriggerId(appId: string | undefined) {
 		const triggerId = Random.id();
-		this.triggersId.set(triggerId, appId);
+		this.appIdByTriggerId.set(triggerId, appId);
 		setTimeout(() => this.invalidateTriggerId(triggerId), ActionManager.TRIGGER_TIMEOUT);
 		return triggerId;
 	}
 
 	public async emitInteraction(appId: string, userInteraction: DistributiveOmit<UiKit.UserInteraction, 'triggerId'>) {
-		this.notifyBusy();
-
 		const triggerId = this.generateTriggerId(appId);
 
-		let timeout: ReturnType<typeof setTimeout> | undefined;
+		return this.runWithTimeout(
+			async () => {
+				let interaction: UiKit.ServerInteraction | undefined;
+
+				try {
+					interaction = (await sdk.rest.post(`/apps/ui.interaction/${appId}`, {
+						...userInteraction,
+						triggerId,
+					})) as UiKit.ServerInteraction;
+
+					this.handleServerInteraction(interaction);
+				} finally {
+					switch (userInteraction.type) {
+						case 'viewSubmit':
+							if (!!interaction && !['errors', 'modal.update', 'contextual_bar.update'].includes(interaction.type))
+								this.disposeView(userInteraction.viewId);
+							break;
+
+						case 'viewClosed':
+							if (!!interaction && interaction.type !== 'errors') this.disposeView(userInteraction.payload.viewId);
+							break;
+					}
+				}
+			},
+			{ triggerId, appId, ...('viewId' in userInteraction ? { viewId: userInteraction.viewId } : {}) },
+		);
+	}
+
+	protected async runWithTimeout<T>(task: () => Promise<T>, details: { triggerId: string; appId: string; viewId?: string }) {
+		this.notifyBusy();
 
-		await Promise.race([
-			new Promise((_, reject) => {
-				timeout = setTimeout(() => reject(new UiKitTriggerTimeoutError('Timeout', { triggerId, appId })), ActionManager.TRIGGER_TIMEOUT);
-			}),
-			sdk.rest
-				.post(`/apps/ui.interaction/${appId}`, {
-					...userInteraction,
-					triggerId,
-				})
-				.then((interaction) => this.handleServerInteraction(interaction)),
-		]).finally(() => {
-			if (timeout) clearTimeout(timeout);
+		let timer: ReturnType<typeof setTimeout> | undefined;
+
+		try {
+			const taskPromise = task();
+			const timeoutPromise = new Promise<T>((_, reject) => {
+				timer = setTimeout(() => {
+					reject(new UiKitTriggerTimeoutError('Timeout', details));
+				}, ActionManager.TRIGGER_TIMEOUT);
+			});
+
+			return await Promise.race([taskPromise, timeoutPromise]);
+		} catch (error) {
+			if (error instanceof UiKitTriggerTimeoutError) {
+				dispatchToastMessage({
+					type: 'error',
+					message: t('UIKit_Interaction_Timeout'),
+				});
+				if (details.viewId) {
+					this.disposeView(details.viewId);
+				}
+			}
+		} finally {
+			if (timer) clearTimeout(timer);
 			this.notifyIdle();
-		});
+		}
 	}
 
-	public handleServerInteraction(interaction: UiKit.ServerInteraction) {
+	public handleServerInteraction(interaction: UiKit.ServerInteraction): UiKit.ServerInteraction['type'] | undefined {
 		const { triggerId } = interaction;
 
-		if (!this.triggersId.has(triggerId)) {
-			return;
-		}
-
 		const appId = this.invalidateTriggerId(triggerId);
 		if (!appId) {
 			return;
@@ -162,8 +199,7 @@ export class ActionManager implements IActionManager {
 
 			case 'banner.close': {
 				const { viewId } = interaction;
-				this.viewInstances.get(viewId)?.close();
-
+				this.disposeView(viewId);
 				break;
 			}
 
@@ -175,9 +211,12 @@ export class ActionManager implements IActionManager {
 
 			case 'contextual_bar.close': {
 				const { view } = interaction;
-				this.viewInstances.get(view.id)?.close();
+				this.disposeView(view.id);
 				break;
 			}
+
+			default:
+				exhaustiveCheck(interaction);
 		}
 
 		return interaction.type;

apps/meteor/client/views/banners/UiKitBanner.tsx

@@ -49,10 +49,6 @@ const UiKitBanner = ({ initialView }: UiKitBannerProps) => {
 			})
 			.catch((error) => {
 				dispatchToastMessage({ type: 'error', message: error });
-				return Promise.reject(error);
-			})
-			.finally(() => {
-				actionManager.disposeView(view.viewId);
 			});
 	});
 

apps/meteor/client/views/modal/uikit/UiKitModal.tsx

@@ -1,4 +1,4 @@
-import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
+import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
 import { UiKitContext } from '@rocket.chat/fuselage-ui-kit';
 import { MarkupInteractionContext } from '@rocket.chat/gazzodown';
 import type * as UiKit from '@rocket.chat/ui-kit';
@@ -22,59 +22,47 @@ const UiKitModal = ({ initialView }: UiKitModalProps) => {
 	const { view, errors, values, updateValues, state } = useUiKitView(initialView);
 	const contextValue = useModalContextValue({ view, values, updateValues });
 
-	const handleSubmit = useMutableCallback((e: FormEvent) => {
+	const handleSubmit = useEffectEvent((e: FormEvent) => {
 		preventSyntheticEvent(e);
-		void actionManager
-			.emitInteraction(view.appId, {
-				type: 'viewSubmit',
-				payload: {
-					view: {
-						...view,
-						state,
-					},
+		void actionManager.emitInteraction(view.appId, {
+			type: 'viewSubmit',
+			payload: {
+				view: {
+					...view,
+					state,
 				},
-				viewId: view.id,
-			})
-			.finally(() => {
-				actionManager.disposeView(view.id);
-			});
+			},
+			viewId: view.id,
+		});
 	});
 
-	const handleCancel = useMutableCallback((e: FormEvent) => {
+	const handleCancel = useEffectEvent((e: FormEvent) => {
 		preventSyntheticEvent(e);
-		void actionManager
-			.emitInteraction(view.appId, {
-				type: 'viewClosed',
-				payload: {
-					viewId: view.id,
-					view: {
-						...view,
-						state,
-					},
-					isCleared: false,
+		void actionManager.emitInteraction(view.appId, {
+			type: 'viewClosed',
+			payload: {
+				viewId: view.id,
+				view: {
+					...view,
+					state,
 				},
-			})
-			.finally(() => {
-				actionManager.disposeView(view.id);
-			});
+				isCleared: false,
+			},
+		});
 	});
 
-	const handleClose = useMutableCallback(() => {
-		void actionManager
-			.emitInteraction(view.appId, {
-				type: 'viewClosed',
-				payload: {
-					viewId: view.id,
-					view: {
-						...view,
-						state,
-					},
-					isCleared: true,
+	const handleClose = useEffectEvent(() => {
+		void actionManager.emitInteraction(view.appId, {
+			type: 'viewClosed',
+			payload: {
+				viewId: view.id,
+				view: {
+					...view,
+					state,
 				},
-			})
-			.finally(() => {
-				actionManager.disposeView(view.id);
-			});
+				isCleared: true,
+			},
+		});
 	});
 
 	return (

apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx

@@ -1,5 +1,5 @@
 import { Avatar, Box, Button, ButtonGroup, ContextualbarFooter, ContextualbarHeader, ContextualbarTitle } from '@rocket.chat/fuselage';
-import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
+import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
 import {
 	UiKitComponent,
 	UiKitContextualBar as UiKitContextualBarSurfaceRender,
@@ -32,63 +32,51 @@ const UiKitContextualBar = ({ initialView }: UiKitContextualBarProps): JSX.Eleme
 
 	const { closeTab } = useRoomToolbox();
 
-	const handleSubmit = useMutableCallback((e: FormEvent) => {
+	const handleSubmit = useEffectEvent((e: FormEvent) => {
 		preventSyntheticEvent(e);
 		closeTab();
-		void actionManager
-			.emitInteraction(view.appId, {
-				type: 'viewSubmit',
-				payload: {
-					view: {
-						...view,
-						state,
-					},
+		void actionManager.emitInteraction(view.appId, {
+			type: 'viewSubmit',
+			payload: {
+				view: {
+					...view,
+					state,
 				},
-				viewId: view.id,
-			})
-			.finally(() => {
-				actionManager.disposeView(view.id);
-			});
+			},
+			viewId: view.id,
+		});
 	});
 
-	const handleCancel = useMutableCallback((e: UIEvent) => {
+	const handleCancel = useEffectEvent((e: UIEvent) => {
 		preventSyntheticEvent(e);
 		closeTab();
-		void actionManager
-			.emitInteraction(view.appId, {
-				type: 'viewClosed',
-				payload: {
-					viewId: view.id,
-					view: {
-						...view,
-						state,
-					},
-					isCleared: false,
+		void actionManager.emitInteraction(view.appId, {
+			type: 'viewClosed',
+			payload: {
+				viewId: view.id,
+				view: {
+					...view,
+					state,
 				},
-			})
-			.finally(() => {
-				actionManager.disposeView(view.id);
-			});
+				isCleared: false,
+			},
+		});
 	});
 
-	const handleClose = useMutableCallback((e: UIEvent) => {
+	const handleClose = useEffectEvent((e: UIEvent) => {
 		preventSyntheticEvent(e);
 		closeTab();
-		void actionManager
-			.emitInteraction(view.appId, {
-				type: 'viewClosed',
-				payload: {
-					viewId: view.id,
-					view: {
-						...view,
-						state,
-					},
-					isCleared: true,
+		void actionManager.emitInteraction(view.appId, {
+			type: 'viewClosed',
+			payload: {
+				viewId: view.id,
+				view: {
+					...view,
+					state,
 				},
-			})
-			.finally(() => {
-				actionManager.disposeView(view.id);
-			});
+				isCleared: true,
+			},
+		});
 	});
 
 	return (

apps/meteor/tests/e2e/app-modal-interaction.spec.ts

@@ -0,0 +1,76 @@
+import { Users } from './fixtures/userStates';
+import { HomeChannel } from './page-objects';
+import { createTargetChannel } from './utils/create-target-channel';
+import { test, expect } from './utils/test';
+
+test.use({ storageState: Users.admin.state });
+
+// TODO: Remove the skip on this test once a fix in the apps engine is done
+// For some reason the app doesn't work properly when installed via insertApp method
+test.describe.skip('app-surfaces-interaction', () => {
+	let poHomeChannel: HomeChannel;
+	let targetChannel: string;
+
+	test.beforeAll(async ({ api }) => {
+		targetChannel = await createTargetChannel(api);
+	});
+
+	test.beforeEach(async ({ page }) => {
+		poHomeChannel = new HomeChannel(page);
+
+		await page.goto('/home');
+	});
+
+	test('expect to submit an success modal', async ({ page }) => {
+		await poHomeChannel.sidenav.openChat(targetChannel);
+		await page.locator('role=button[name="Options"]').click();
+		await page.locator('[data-key="success"]').click();
+		await page.locator('role=button[name="success"]').click();
+
+		const updatedButton = page.locator('role=button[name="success"]');
+		await expect(updatedButton).not.toBeVisible();
+	});
+
+	test('expect to not close the modal and there is an error in the modal', async ({ page }) => {
+		await poHomeChannel.sidenav.openChat(targetChannel);
+		await page.locator('role=button[name="Options"]').click();
+		await page.locator('[data-key="error"]').click();
+		await page.locator('role=button[name="error"]').click();
+
+		const updatedTitle = page.locator('role=button[name="error"]');
+		expect(updatedTitle).toBeDefined();
+
+		const input = page.locator('input[type="text"]');
+
+		await input.click();
+		await input.fill('fixed');
+
+		await page.locator('role=button[name="error"]').click();
+		await expect(input).not.toBeVisible();
+	});
+
+	test('expect to show the toaster error for modal that timeout the execution', async ({ page }) => {
+		await poHomeChannel.sidenav.openChat(targetChannel);
+		await page.locator('role=button[name="Options"]').click();
+		await page.locator('[data-key="timeout"]').click();
+		await page.locator('role=button[name="timeout"]').click();
+		await page.locator('role=alert').waitFor();
+
+		const toaster = page.locator('role=alert');
+
+		await expect(toaster).toBeVisible();
+	});
+
+	test('expect change the modal and then submit the updated modal', async ({ page }) => {
+		await poHomeChannel.sidenav.openChat(targetChannel);
+		await page.locator('role=button[name="Options"]').click();
+		await page.locator('[data-key="update"]').click();
+		await page.locator('role=button[name="update"]').click();
+
+		const updatedTitle = page.locator('role=button[name="title updated"]');
+		expect(updatedTitle).toBeDefined();
+
+		const updatedButton = page.locator('role=button[name="updated"]');
+		expect(updatedButton).toBeDefined();
+	});
+});

packages/ui-contexts/src/ActionManagerContext.ts

@@ -13,7 +13,7 @@ export interface IActionManager {
 	notifyBusy(): void;
 	notifyIdle(): void;
 	generateTriggerId(appId: string | undefined): string;
-	emitInteraction(appId: string, userInteraction: DistributiveOmit<UiKit.UserInteraction, 'triggerId'>): Promise<unknown>;
+	emitInteraction(appId: string, userInteraction: DistributiveOmit<UiKit.UserInteraction, 'triggerId'>): Promise<void>;
 	handleServerInteraction(interaction: UiKit.ServerInteraction): UiKit.ServerInteraction['type'] | undefined;
 	getInteractionPayloadByViewId(viewId: UiKit.ContextualBarView['id']):
 		| {