[#237] fixes playing audio

src/client/common/AudioPlayer.ts

@@ -1,8 +1,15 @@
-const playing = new Map<string, HTMLAudioElement>()
+const cachedAudio = new Map<string, HTMLAudioElement>()
+
+function stop(audio?: HTMLAudioElement): void {
+  if (audio) {
+    audio.pause()
+    audio.currentTime = 0
+  }
+}
 
 export function playAudio(src: string, onStop?: () => void): Promise<void> {
-  const audio = playing.get(src) || new Audio(src)
-  playing.set(src, audio)
+  const audio = cachedAudio.get(src) || new Audio(src)
+  cachedAudio.set(src, audio)
   if (onStop) {
     audio.addEventListener('ended', onStop)
     audio.addEventListener('pause', onStop)
@@ -11,15 +18,18 @@ export function playAudio(src: string, onStop?: () => void): Promise<void> {
 }
 
 export function stopAudio(src: string) {
-  const audio = playing.get(src)
-  if (audio) {
-    audio.pause()
-    audio.currentTime = 0
+  const audio = cachedAudio.get(src)
+  stop(audio)
+}
+
+export function stopAnyPlayingAudio() {
+  for (const audio of cachedAudio.values()) {
+    stop(audio)
   }
 }
 
 export function anyAudioPlaying(): boolean {
-  for (const audio of playing.values()) {
+  for (const audio of cachedAudio.values()) {
     if (!audio.paused) {
       return true
     }
@@ -28,5 +38,5 @@ export function anyAudioPlaying(): boolean {
 }
 
 export function deleteAudio(src: string) {
-  playing.delete(src)
+  cachedAudio.delete(src)
 }

src/client/monitor/Monitor.test.tsx

@@ -150,11 +150,11 @@ it('should show feed errors', async () => {
 
 it('should trigger notifications and stop any audio notifications if user leaves the page', () => {
   const feedId = 'some-tray-id'
-  jest.spyOn(NotificationsHook, 'useNotifications').mockReturnValueOnce('some-sfx.mp3')
+  jest.spyOn(NotificationsHook, 'useNotifications').mockReturnValueOnce()
   jest.spyOn(Gateway, 'post').mockReturnValueOnce(fakeRequest([
     buildProjectApi({trayId: feedId, prognosis: Prognosis.sick})
   ]))
-  jest.spyOn(AudioPlayer, 'stopAudio')
+  jest.spyOn(AudioPlayer, 'stopAnyPlayingAudio')
   const state = {
     [FEEDS_ROOT]: {
       [feedId]: buildFeed({trayId: feedId})
@@ -167,5 +167,5 @@ it('should trigger notifications and stop any audio notifications if user leaves
 
   unmount()
 
-  expect(AudioPlayer.stopAudio).toHaveBeenCalledWith('some-sfx.mp3')
+  expect(AudioPlayer.stopAnyPlayingAudio).toHaveBeenCalled()
 })

src/client/monitor/Monitor.tsx

@@ -14,7 +14,7 @@ import screenfull from 'screenfull'
 import {useNevergreenContext} from '../Nevergreen'
 import {useInterestingProjects} from './InterestingProjectsHook'
 import {useNotifications} from './notifications/NotificationsHook'
-import {stopAudio} from '../common/AudioPlayer'
+import {stopAnyPlayingAudio} from '../common/AudioPlayer'
 
 export function Monitor(): ReactElement {
   const {menusHidden, toggleMenusHidden} = useNevergreenContext()
@@ -31,13 +31,13 @@ export function Monitor(): ReactElement {
   const feedsAdded = !isEmpty(feeds)
 
   const {loaded, projects, feedErrors} = useInterestingProjects()
-  const sfx = useNotifications(projects, feedErrors)
+  useNotifications(projects, feedErrors)
 
   useEffect(() => {
-    if (sfx) {
-      stopAudio(sfx)
+    return () => {
+      stopAnyPlayingAudio()
     }
-  }, [sfx])
+  }, [])
 
   useShortcut('f', () => {
     if (screenfull.isEnabled && ref.current) {

src/client/monitor/notifications/AudioPlayerHook.ts

@@ -0,0 +1,19 @@
+import {useEffect} from 'react'
+import {isNotBlank} from '../../common/Utils'
+import {playAudio, stopAudio} from '../../common/AudioPlayer'
+import {error} from '../../common/Logger'
+
+export function usePlayAudio(src: string): void {
+  useEffect(() => {
+    if (isNotBlank(src)) {
+      try {
+        void playAudio(src)
+      } catch (e) {
+        error('Unable to play audio', e)
+      }
+    }
+    return () => {
+      stopAudio(src)
+    }
+  }, [src])
+}

src/client/monitor/notifications/NotificationsHook.test.tsx

@@ -8,6 +8,7 @@ import {useNotifications} from './NotificationsHook'
 import {FeedErrors} from '../../domain/FeedError'
 import * as AudioPlayer from '../../common/AudioPlayer'
 import capitalize from 'lodash/capitalize'
+import {waitFor} from '@testing-library/react'
 
 interface PrognosisTest {
   readonly previous: ProjectPrognosis,
@@ -217,8 +218,6 @@ describe('audio notifications', () => {
     ${Prognosis.healthyBuilding} | ${Prognosis.healthy}
     ${Prognosis.healthyBuilding} | ${Prognosis.unknown}
   `('should play notification for transition $previous -> $current', ({previous, current}: PrognosisTest) => {
-    jest.spyOn(AudioPlayer, 'playAudio').mockResolvedValue()
-
     const state = {
       [NOTIFICATIONS_ROOT]: {
         allowAudioNotifications: true,
@@ -275,7 +274,7 @@ describe('audio notifications', () => {
   })
 
   it('should not play more notifications if a previous notification is still playing', () => {
-    jest.spyOn(AudioPlayer, 'anyAudioPlaying').mockReturnValue(false)
+    jest.spyOn(AudioPlayer, 'anyAudioPlaying').mockReturnValueOnce(false)
 
     const state = {
       [NOTIFICATIONS_ROOT]: {
@@ -336,9 +335,7 @@ describe('audio notifications', () => {
     expect(AudioPlayer.playAudio).not.toHaveBeenCalled()
   })
 
-  it('should not play audio notification if nothing has changed', () => {
-    jest.spyOn(AudioPlayer, 'playAudio').mockResolvedValue()
-
+  it('should not play audio notification if nothing has changed', async () => {
     const state = {
       [NOTIFICATIONS_ROOT]: {
         allowAudioNotifications: true,
@@ -368,7 +365,9 @@ describe('audio notifications', () => {
     const {rerender} = render(<HookWrapper projects={projectsFirstFetch} errors={errors}/>, {state})
     rerender(<HookWrapper projects={projectsSecondFetch} errors={errors}/>)
 
-    expect(AudioPlayer.playAudio).toHaveBeenCalledTimes(1)
+    await waitFor(() => {
+      expect(AudioPlayer.playAudio).toHaveBeenCalledTimes(1)
+    })
   })
 })
 

src/client/monitor/notifications/NotificationsHook.ts

@@ -6,11 +6,10 @@ import {
   getNotifications
 } from '../../settings/notifications/NotificationsReducer'
 import {sendSystemNotification} from '../../common/SystemNotifications'
-import {useEffect, useRef, useState} from 'react'
+import {useEffect, useState} from 'react'
 import {isNotBlank} from '../../common/Utils'
-import {anyAudioPlaying, playAudio} from '../../common/AudioPlayer'
+import {anyAudioPlaying} from '../../common/AudioPlayer'
 import {FeedError, FeedErrors} from '../../domain/FeedError'
-import {error} from '../../common/Logger'
 import healthyIcon from './healthy.png'
 import unknownIcon from './unknown.png'
 import healthyBuildingIcon from './healthy-building.png'
@@ -21,6 +20,7 @@ import groupBy from 'lodash/groupBy'
 import capitalize from 'lodash/capitalize'
 import {notificationIcons} from '../../settings/notifications/icons/NotificationIcon'
 import {UpdateBrowserTitleHookProps, useUpdateBrowserTitle} from '../../common/BrowserTitleHook'
+import {usePlayAudio} from './AudioPlayerHook'
 
 const systemNotificationIcons = {
   [Prognosis.healthy]: healthyIcon,
@@ -44,13 +44,13 @@ function justTransitionedTo(project: Project | FeedError, prognosis: Prognosis):
   return project.prognosis === prognosis && project.previousPrognosis !== prognosis
 }
 
-function body(projects: ReadonlyArray<Project | FeedError>): string {
+function notificationBody(projects: ReadonlyArray<Project | FeedError>): string {
   return projects
     .map(({description}) => description)
     .join(', ')
 }
 
-function title(prognosis: Prognosis, total: number): string {
+function notificationTitle(prognosis: Prognosis, total: number): string {
   if (Prognosis.error === prognosis) {
     return total === 1
       ? 'feed error!'
@@ -62,19 +62,21 @@ function title(prognosis: Prognosis, total: number): string {
   }
 }
 
-export function useNotifications(projects: Projects, feedErrors: FeedErrors): string | undefined {
+export function useNotifications(projects: Projects, feedErrors: FeedErrors): void {
   const notifications = useSelector(getNotifications)
   const allowSystemNotifications = useSelector(getAllowSystemNotifications)
   const allowAudioNotifications = useSelector(getAllowAudioNotifications)
-  const sfxSrc = useRef<string>()
+  const [sfxSrc, setSfxSrc] = useState('')
   const [browserTitle, setBrowserTitle] = useState<UpdateBrowserTitleHookProps>({title: 'Monitor'})
 
   useUpdateBrowserTitle(browserTitle)
+  usePlayAudio(sfxSrc)
 
   useEffect(() => {
-    sfxSrc.current = undefined
     const all = [...feedErrors, ...projects]
 
+    setSfxSrc('')
+
     Object.entries(groupBy(all, 'prognosis'))
       .sort(([aPrognosis], [bPrognosis]) => {
         return prognosisPriority.indexOf(aPrognosis as Prognosis) - prognosisPriority.indexOf(bPrognosis as Prognosis)
@@ -95,14 +97,14 @@ export function useNotifications(projects: Projects, feedErrors: FeedErrors): st
 
             if (shouldSendSystemNotification) {
               void sendSystemNotification({
-                title: title(prognosis, projectsToAlert.length),
-                body: body(projectsToAlert),
+                title: notificationTitle(prognosis, projectsToAlert.length),
+                body: notificationBody(projectsToAlert),
                 icon: systemNotificationIcons[prognosis]
               })
             }
 
             if (shouldPlayAudio) {
-              sfxSrc.current = notification.sfx
+              setSfxSrc(notification.sfx)
             }
           }
         }
@@ -112,15 +114,5 @@ export function useNotifications(projects: Projects, feedErrors: FeedErrors): st
           faviconHref: notificationIcons[prognosis]
         })
       })
-
-    if (sfxSrc.current) {
-      try {
-        void playAudio(sfxSrc.current)
-      } catch (e) {
-        error('Unable to play audio notification', e)
-      }
-    }
   }, [projects, feedErrors, notifications, allowSystemNotifications, allowAudioNotifications])
-
-  return sfxSrc.current
 }