diff --git a/.changeset/young-candles-explode.md b/.changeset/young-candles-explode.md new file mode 100644 index 00000000000..91ff2458a40 --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/ui-message/client/ActionManager.ts b/apps/meteor/app/ui-message/client/ActionManager.ts index 733664affd1..4c892d6d32f 100644 --- a/apps/meteor/app/ui-message/client/ActionManager.ts +++ b/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(); + protected appIdByTriggerId = new Map(); protected viewInstances = new Map< string, @@ -35,8 +38,8 @@ export class ActionManager implements IActionManager { public constructor(protected router: ContextType) {} 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) { - this.notifyBusy(); - const triggerId = this.generateTriggerId(appId); - let timeout: ReturnType | 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(task: () => Promise, 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 | undefined; + + try { + const taskPromise = task(); + const timeoutPromise = new Promise((_, 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; diff --git a/apps/meteor/client/views/banners/UiKitBanner.tsx b/apps/meteor/client/views/banners/UiKitBanner.tsx index 27478422392..72d2e5a9605 100644 --- a/apps/meteor/client/views/banners/UiKitBanner.tsx +++ b/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); }); }); diff --git a/apps/meteor/client/views/modal/uikit/UiKitModal.tsx b/apps/meteor/client/views/modal/uikit/UiKitModal.tsx index 1ce3efb28c5..56242d399ac 100644 --- a/apps/meteor/client/views/modal/uikit/UiKitModal.tsx +++ b/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 ( diff --git a/apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx b/apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx index d32e05f6aeb..f06ff50842d 100644 --- a/apps/meteor/client/views/room/contextualBar/uikit/UiKitContextualBar.tsx +++ b/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 ( diff --git a/apps/meteor/tests/e2e/app-modal-interaction.spec.ts b/apps/meteor/tests/e2e/app-modal-interaction.spec.ts new file mode 100644 index 00000000000..1a1ce2b656e --- /dev/null +++ b/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(); + }); +}); diff --git a/packages/ui-contexts/src/ActionManagerContext.ts b/packages/ui-contexts/src/ActionManagerContext.ts index 0847094bb4d..ed5c510a5e8 100644 --- a/packages/ui-contexts/src/ActionManagerContext.ts +++ b/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): Promise; + emitInteraction(appId: string, userInteraction: DistributiveOmit): Promise; handleServerInteraction(interaction: UiKit.ServerInteraction): UiKit.ServerInteraction['type'] | undefined; getInteractionPayloadByViewId(viewId: UiKit.ContextualBarView['id']): | {