diff --git a/resources/electron/electron-plugin/dist/server/api/notification.js b/resources/electron/electron-plugin/dist/server/api/notification.js index 44694be4..eebf3788 100644 --- a/resources/electron/electron-plugin/dist/server/api/notification.js +++ b/resources/electron/electron-plugin/dist/server/api/notification.js @@ -2,6 +2,7 @@ import { Notification } from 'electron'; import express from 'express'; import fs from 'fs'; import playSoundLib from 'play-sound'; +import state from '../state.js'; import { broadcastToWindows, notifyLaravel } from '../utils.js'; const isLocalFile = (sound) => { if (typeof sound !== 'string') @@ -45,6 +46,7 @@ router.post('/', (req, res) => { }); } notification.on('click', (event) => { + delete state.notifications[notificationReference]; notifyLaravel('events', { event: eventName || '\\Native\\Desktop\\Events\\Notifications\\NotificationClicked', payload: { @@ -74,6 +76,7 @@ router.post('/', (req, res) => { }); }); notification.on('close', (event) => { + delete state.notifications[notificationReference]; notifyLaravel('events', { event: '\\Native\\Desktop\\Events\\Notifications\\NotificationClosed', payload: { @@ -82,6 +85,7 @@ router.post('/', (req, res) => { }, }); }); + state.notifications[notificationReference] = notification; notification.show(); res.status(200).json({ reference: notificationReference, diff --git a/resources/electron/electron-plugin/dist/server/state.js b/resources/electron/electron-plugin/dist/server/state.js index e028d68e..cde26449 100644 --- a/resources/electron/electron-plugin/dist/server/state.js +++ b/resources/electron/electron-plugin/dist/server/state.js @@ -36,6 +36,7 @@ export default { randomSecret: generateRandomString(32), processes: {}, windows: {}, + notifications: {}, noFocusOnRestart: false, findWindow(id) { return this.windows[id] || null; diff --git a/resources/electron/electron-plugin/src/server/api/notification.ts b/resources/electron/electron-plugin/src/server/api/notification.ts index 6c752393..15f70ff5 100644 --- a/resources/electron/electron-plugin/src/server/api/notification.ts +++ b/resources/electron/electron-plugin/src/server/api/notification.ts @@ -2,6 +2,7 @@ import { Notification } from 'electron'; import express from 'express'; import fs from 'fs'; import playSoundLib from 'play-sound'; +import state from '../state.js'; import { broadcastToWindows, notifyLaravel } from '../utils.js'; const isLocalFile = (sound: unknown) => { @@ -69,6 +70,7 @@ router.post('/', (req, res) => { } notification.on('click', (event) => { + delete state.notifications[notificationReference]; notifyLaravel('events', { event: eventName || '\\Native\\Desktop\\Events\\Notifications\\NotificationClicked', payload: { @@ -101,6 +103,7 @@ router.post('/', (req, res) => { }); notification.on('close', (event) => { + delete state.notifications[notificationReference]; notifyLaravel('events', { event: '\\Native\\Desktop\\Events\\Notifications\\NotificationClosed', payload: { @@ -110,6 +113,14 @@ router.post('/', (req, res) => { }); }); + // Electron only retains a weak reference to main-process Notification + // objects. Without a strong JS reference the wrapper can be garbage + // collected before the user interacts with it, after which the click / + // action / reply / close handlers silently never fire (most visible on + // macOS when the app is idle or backgrounded). Keep it reachable until it + // is dismissed. See https://github.com/electron/electron/issues/16922 + state.notifications[notificationReference] = notification; + notification.show(); res.status(200).json({ diff --git a/resources/electron/electron-plugin/src/server/state.ts b/resources/electron/electron-plugin/src/server/state.ts index e78caa05..580ceeb1 100644 --- a/resources/electron/electron-plugin/src/server/state.ts +++ b/resources/electron/electron-plugin/src/server/state.ts @@ -1,4 +1,4 @@ -import { BrowserWindow, Tray, UtilityProcess } from 'electron'; +import { BrowserWindow, Notification, Tray, UtilityProcess } from 'electron'; import Store from 'electron-store'; import type { Menubar } from '../libs/menubar/index.js'; import { notifyLaravel } from './utils.js'; @@ -31,6 +31,7 @@ interface State { icon: string | null; processes: Record }>; windows: Record; + notifications: Record; randomSecret: string; store: Store; findWindow: (id: string) => BrowserWindow | null; @@ -64,6 +65,7 @@ export default { randomSecret: generateRandomString(32), processes: {}, windows: {}, + notifications: {}, noFocusOnRestart: false, findWindow(id: string) { return this.windows[id] || null; diff --git a/resources/electron/electron-plugin/tests/notification.test.ts b/resources/electron/electron-plugin/tests/notification.test.ts new file mode 100644 index 00000000..d6a567a2 --- /dev/null +++ b/resources/electron/electron-plugin/tests/notification.test.ts @@ -0,0 +1,205 @@ +import axios from 'axios'; +import { Notification } from 'electron'; +import express from 'express'; +import fs from 'fs'; +import type { Server } from 'http'; +import type { AddressInfo } from 'net'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import notificationRoutes from '../src/server/api/notification'; +import state from '../src/server/state'; +import { broadcastToWindows, notifyLaravel } from '../src/server/utils'; + +const { playMock } = vi.hoisted(() => ({ playMock: vi.fn() })); + +// The route constructs a real electron Notification, so mock the class with one +// that records its event handlers and lets the test fire them, mimicking the +// OS raising a click/close on the notification. +vi.mock('electron', () => ({ + Notification: vi.fn().mockImplementation(() => { + const handlers: Record void> = {}; + return { + show: vi.fn(), + on: vi.fn((event: string, callback: (...args: any[]) => void) => { + handlers[event] = callback; + }), + emit: (event: string, ...args: any[]) => handlers[event]?.(...args), + }; + }), +})); + +vi.mock('play-sound', () => ({ default: vi.fn(() => ({ play: playMock })) })); + +vi.mock('../src/server/utils.js', () => ({ + notifyLaravel: vi.fn(), + broadcastToWindows: vi.fn(), +})); + +let server: Server; + +const latestNotification = () => vi.mocked(Notification).mock.results.at(-1)!.value; + +const mockFsAccess = (error: Error | null = null) => + vi.spyOn(fs, 'access').mockImplementation(((_path: any, _mode: any, callback: any) => callback(error)) as any); + +describe('notification', () => { + beforeEach(async () => { + vi.clearAllMocks(); + state.notifications = {}; + + const app = express(); + app.use(express.json()); + app.use('/api/notification', notificationRoutes); + + await new Promise((resolve) => { + server = app.listen(0, '127.0.0.1', () => resolve()); + }); + + const { port } = server.address() as AddressInfo; + axios.defaults.baseURL = `http://127.0.0.1:${port}`; + }); + + afterEach(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + it('constructs the notification with the provided options', async () => { + await axios.post('/api/notification', { + title: 'Build finished', + body: 'Tests are green', + subtitle: 'My App', + }); + + expect(Notification).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Build finished', + body: 'Tests are green', + subtitle: 'My App', + }), + ); + expect(latestNotification().show).toHaveBeenCalledOnce(); + }); + + it('retains the notification so its event handlers survive garbage collection', async () => { + const response = await axios.post('/api/notification', { + title: 'Build finished', + body: 'Tests are green', + reference: 'process-42', + }); + + expect(response.status).toBe(200); + expect(response.data.reference).toBe('process-42'); + // Held in state, mirroring windows/processes, so V8 cannot collect the + // wrapper before the user interacts with the OS notification. + expect(state.notifications['process-42']).toBe(latestNotification()); + }); + + it('retains notifications created without an explicit reference', async () => { + const response = await axios.post('/api/notification', { + title: 'Heads up', + }); + + expect(state.notifications[response.data.reference]).toBe(latestNotification()); + }); + + it('releases the notification and forwards the click to Laravel', async () => { + await axios.post('/api/notification', { + title: 'Build finished', + reference: 'process-42', + event: '\\App\\Events\\Terminals\\ProcessNotificationClicked', + }); + + latestNotification().emit('click', {}); + + expect(notifyLaravel).toHaveBeenCalledWith('events', { + event: '\\App\\Events\\Terminals\\ProcessNotificationClicked', + payload: { reference: 'process-42', event: JSON.stringify({}) }, + }); + expect(state.notifications['process-42']).toBeUndefined(); + }); + + it('falls back to the default clicked event when none is provided', async () => { + await axios.post('/api/notification', { + title: 'Build finished', + reference: 'process-42', + }); + + latestNotification().emit('click', {}); + + expect(notifyLaravel).toHaveBeenCalledWith('events', { + event: '\\Native\\Desktop\\Events\\Notifications\\NotificationClicked', + payload: { reference: 'process-42', event: JSON.stringify({}) }, + }); + }); + + it('forwards notification actions to Laravel', async () => { + await axios.post('/api/notification', { + title: 'Build finished', + reference: 'process-42', + }); + + latestNotification().emit('action', {}, 1); + + expect(notifyLaravel).toHaveBeenCalledWith('events', { + event: '\\Native\\Desktop\\Events\\Notifications\\NotificationActionClicked', + payload: { reference: 'process-42', index: 1, event: JSON.stringify({}) }, + }); + }); + + it('forwards notification replies to Laravel', async () => { + await axios.post('/api/notification', { + title: 'Build finished', + reference: 'process-42', + }); + + latestNotification().emit('reply', {}, 'on it'); + + expect(notifyLaravel).toHaveBeenCalledWith('events', { + event: '\\Native\\Desktop\\Events\\Notifications\\NotificationReply', + payload: { reference: 'process-42', reply: 'on it', event: JSON.stringify({}) }, + }); + }); + + it('releases the notification and forwards the close to Laravel', async () => { + await axios.post('/api/notification', { + title: 'Build finished', + reference: 'process-42', + }); + + latestNotification().emit('close', {}); + + expect(notifyLaravel).toHaveBeenCalledWith('events', { + event: '\\Native\\Desktop\\Events\\Notifications\\NotificationClosed', + payload: { reference: 'process-42', event: JSON.stringify({}) }, + }); + expect(state.notifications['process-42']).toBeUndefined(); + }); + + it('plays a local sound file itself and mutes electron for it', async () => { + mockFsAccess(); + + await axios.post('/api/notification', { + title: 'Build finished', + sound: '/sounds/done.mp3', + }); + + // Electron must not also play it, so the sound is stripped and silenced. + expect(Notification).toHaveBeenCalledWith(expect.objectContaining({ sound: undefined, silent: true })); + expect(playMock).toHaveBeenCalledWith('/sounds/done.mp3', expect.any(Function)); + }); + + it('logs an error and does not play when the local sound file is missing', async () => { + mockFsAccess(new Error('missing')); + + await axios.post('/api/notification', { + title: 'Build finished', + sound: '/sounds/missing.mp3', + }); + + expect(broadcastToWindows).toHaveBeenCalledWith('log', { + level: 'error', + message: 'Sound file not found: /sounds/missing.mp3', + context: { sound: '/sounds/missing.mp3' }, + }); + expect(playMock).not.toHaveBeenCalled(); + }); +});