From d45e30e192e6ea2e1e899cc8750c4dc21c4adf94 Mon Sep 17 00:00:00 2001 From: Jean Brito Date: Fri, 21 Aug 2026 13:20:30 -0300 Subject: [PATCH] fix: desktop notifications force-closed 10s after being shown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client scheduled an auto-close from a fallback left behind when the 'Notification Duration' setting was removed in #15737: the server never sends 'duration' for desktop notifications, so every notification was closed after exactly 10 seconds. That close cannot remove an OS notification the platform still shows — a Windows Action Center card stays repliable indefinitely — so its only effect was to tell the app the notification was over while the user could still act on it, which could drop late quick replies. Only schedule the timer when the server actually provides a duration. --- .../quick-notifications-stop-auto-closing.md | 5 + .../notification/useNotification.spec.ts | 161 ++++++++++++++++++ .../hooks/notification/useNotification.ts | 2 +- 3 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 .changeset/quick-notifications-stop-auto-closing.md create mode 100644 apps/meteor/client/hooks/notification/useNotification.spec.ts 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); }