diff --git a/.changeset/quick-notifications-stop-auto-closing.md b/.changeset/quick-notifications-stop-auto-closing.md new file mode 100644 index 0000000000000..52ca614f1a13b --- /dev/null +++ b/.changeset/quick-notifications-stop-auto-closing.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes desktop notifications being force-closed 10 seconds after being shown, even though the server never requests a duration for them. The forced close only told the app the notification was finished while the OS could still display and interact with it (for example, quick-replying from a Windows Action Center card), which could cause late replies to be silently dropped. Desktop notifications now only auto-close when the server explicitly provides a duration. diff --git a/apps/meteor/client/hooks/notification/useNotification.spec.ts b/apps/meteor/client/hooks/notification/useNotification.spec.ts new file mode 100644 index 0000000000000..38c7e79f1b45f --- /dev/null +++ b/apps/meteor/client/hooks/notification/useNotification.spec.ts @@ -0,0 +1,161 @@ +import type { INotificationDesktop } from '@rocket.chat/core-typings'; +import { mockAppRoot } from '@rocket.chat/mock-providers'; +import { renderHook } from '@testing-library/react'; + +import { useNotification } from './useNotification'; +import { useNotificationAllowed } from './useNotificationAllowed'; +import { onClientMessageReceived } from '../../lib/onClientMessageReceived'; + +jest.mock('./useNotificationAllowed', () => ({ + useNotificationAllowed: jest.fn(), +})); + +jest.mock('../../lib/onClientMessageReceived', () => ({ + onClientMessageReceived: jest.fn(), +})); + +jest.mock('../../../app/utils/client/lib/SDKClient', () => ({ + sdk: { + rest: { + post: jest.fn(), + }, + }, +})); + +jest.mock('../../../app/utils/client', () => ({ + getUserAvatarURL: jest.fn(), +})); + +type NotificationEventListener = (event: { response: string }) => void; + +class MockNotification { + static permission: NotificationPermission = 'granted'; + + static listenersByInstance: NotificationEventListener[] = []; + + static instances: MockNotification[] = []; + + title: string; + + options: NotificationOptions | undefined; + + onclick: (() => void) | null = null; + + constructor(title: string, options?: NotificationOptions) { + this.title = title; + this.options = options; + MockNotification.instances.push(this); + } + + close = jest.fn(); + + addEventListener(type: 'reply', listener: NotificationEventListener): void { + if (type === 'reply') { + MockNotification.listenersByInstance.push(listener); + } + } +} + +const buildPayload = (tmid?: string, duration?: number): INotificationDesktop => ({ + title: 'title', + text: 'text', + ...(duration !== undefined && { duration }), + payload: { + _id: 'msgId', + rid: 'roomId', + ...(tmid && { tmid }), + sender: { _id: 'senderId', username: 'sender' }, + type: 'c', + name: 'roomName', + message: { msg: 'text' }, + audioNotificationValue: 'default', + }, +}); + +describe('useNotification', () => { + const originalNotification = window.Notification; + + beforeEach(() => { + jest.clearAllMocks(); + MockNotification.listenersByInstance = []; + MockNotification.instances = []; + (window as any).Notification = MockNotification; + (useNotificationAllowed as jest.MockedFunction).mockReturnValue(true); + (onClientMessageReceived as jest.MockedFunction).mockImplementation((message: any) => + Promise.resolve(message), + ); + }); + + afterAll(() => { + (window as any).Notification = originalNotification; + }); + + describe('auto-close timer', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('does not schedule an auto-close timer when the server does not provide a duration', async () => { + const { result } = renderHook(() => useNotification(), { + wrapper: mockAppRoot().build(), + }); + + await result.current(buildPayload()); + + const [instance] = MockNotification.instances; + jest.advanceTimersByTime(60_000); + + expect(instance.close).not.toHaveBeenCalled(); + }); + + it('honours a server-provided duration and closes the notification after it elapses', async () => { + const { result } = renderHook(() => useNotification(), { + wrapper: mockAppRoot().build(), + }); + + await result.current(buildPayload(undefined, 5)); + + const [instance] = MockNotification.instances; + + jest.advanceTimersByTime(4_999); + expect(instance.close).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(1); + expect(instance.close).toHaveBeenCalledTimes(1); + }); + + it('leaves a notification without a duration open indefinitely, so a late quick reply can still reach it', async () => { + const { result } = renderHook(() => useNotification(), { + wrapper: mockAppRoot().build(), + }); + + await result.current(buildPayload()); + + const [instance] = MockNotification.instances; + + // Desktop clients keep such a notification actionable (a Windows Action + // Center card stays repliable), so the client must not declare it over. + jest.advanceTimersByTime(10 * 60_000); + + expect(instance.close).not.toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(0); + }); + + it('does not schedule an auto-close timer when requireInteraction is set, even with a duration', async () => { + const { result } = renderHook(() => useNotification(), { + wrapper: mockAppRoot().withUserPreference('desktopNotificationRequireInteraction', true).build(), + }); + + await result.current(buildPayload(undefined, 5)); + + const [instance] = MockNotification.instances; + jest.advanceTimersByTime(60_000); + + expect(instance.close).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/meteor/client/hooks/notification/useNotification.ts b/apps/meteor/client/hooks/notification/useNotification.ts index 88296e3334323..6453cea5fcb2f 100644 --- a/apps/meteor/client/hooks/notification/useNotification.ts +++ b/apps/meteor/client/hooks/notification/useNotification.ts @@ -42,7 +42,7 @@ export const useNotification = () => { } as NotificationOptions & { canReply?: boolean; }); - const notificationDuration = !requireInteraction ? (notification.duration ?? 0) - 0 || 10 : -1; + const notificationDuration = !requireInteraction && notification.duration ? notification.duration - 0 : 0; if (notificationDuration > 0) { setTimeout(() => n.close(), notificationDuration * 1000); }