From 5eb22bc0d2864bd493b8909c319d7cabaa89e514 Mon Sep 17 00:00:00 2001 From: William Craig Date: Tue, 19 Nov 2024 14:10:58 +0000 Subject: [PATCH 1/4] VIH-11144 update verify audio recording stream (#2303) * - Deleted the timed interval check of verifyAudioRecordingStream, and trigger it after countdown complete event. - Fixed translations for buttons - added disable a function timeout to protect against spamming the pause resume buttons * removed redundant test * fix test * test coverage * added back property accidentally removed * lint --- .../services/audio-recording.service.spec.ts | 19 ++- .../app/services/audio-recording.service.ts | 55 +++++---- .../judge-waiting-room.component.spec.ts | 113 ++++-------------- .../judge-waiting-room.component.ts | 92 ++++++-------- ...-consultation-room-controls.component.html | 4 +- ...nsultation-room-controls.component.spec.ts | 42 +++++-- ...te-consultation-room-controls.component.ts | 26 ++-- .../ClientApp/src/assets/i18n/cy.json | 4 +- .../ClientApp/src/assets/i18n/en.json | 4 +- 9 files changed, 163 insertions(+), 196 deletions(-) diff --git a/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.spec.ts b/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.spec.ts index e9a38fe50c..01447acbcd 100644 --- a/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.spec.ts +++ b/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.spec.ts @@ -83,7 +83,7 @@ describe('AudioRecordingService', () => { describe('reconnectToWowza', () => { it('should reconnect to Wowza', async () => { const failedToConnectCallback = jasmine.createSpy('failedToConnectCallback'); - service.conference = { id: 'conferenceId', audioRecordingIngestUrl: 'ingestUrl' } as any; + service.conference = { id: globalConference.id, audioRecordingIngestUrl: 'ingestUrl' } as any; videoCallServiceSpy.connectWowzaAgent.and.callFake((url, callback) => { callback({ status: 'success', result: ['newUUID'] }); }); @@ -91,7 +91,7 @@ describe('AudioRecordingService', () => { await service.reconnectToWowza(failedToConnectCallback); expect(service.restartActioned).toBeTrue(); expect(videoCallServiceSpy.connectWowzaAgent).toHaveBeenCalledWith('ingestUrl', jasmine.any(Function)); - expect(eventServiceSpy.sendAudioRecordingPaused).toHaveBeenCalledWith('conferenceId', false); + expect(eventServiceSpy.sendAudioRecordingPaused).toHaveBeenCalledWith(globalConference.id, false); expect(failedToConnectCallback).not.toHaveBeenCalled(); }); @@ -106,6 +106,21 @@ describe('AudioRecordingService', () => { expect(failedToConnectCallback).toHaveBeenCalled(); }); + it('should call push false to wowzaAgentConnection$ if reconnect to Wowza fails', async () => { + service.conference = { id: 'conferenceId', audioRecordingIngestUrl: 'ingestUrl' } as any; + videoCallServiceSpy.connectWowzaAgent.and.callFake((url, callback) => { + callback({ status: 'failure' }); + }); + + let emittedValue: boolean | undefined; + service.getWowzaAgentConnectionState().subscribe(value => (emittedValue = value)); + + await service.reconnectToWowza(); + + // Assert that `false` was emitted by the observable + expect(emittedValue).toBe(false); + }); + it('should clean up dial out connections', () => { service.dialOutUUID = ['uuid1', 'uuid2']; service.cleanupDialOutConnections(); diff --git a/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.ts b/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.ts index c0550af74c..6cb2fe08e1 100644 --- a/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.ts +++ b/VideoWeb/VideoWeb/ClientApp/src/app/services/audio-recording.service.ts @@ -43,18 +43,7 @@ export class AudioRecordingService { takeUntil(this.onDestroy$), distinctUntilChanged((prev, curr) => prev?.uuid === curr?.uuid) ) - .subscribe(participant => { - if (participant) { - this.dialOutUUID = [...new Set([...this.dialOutUUID, participant?.uuid])]; - } - this.wowzaAgent = participant; - if (participant?.isAudioOnlyCall) { - this.wowzaAgentConnection$.next(true); - this.restartActioned = false; - } else { - this.wowzaAgentConnection$.next(false); - } - }); + .subscribe(wowzaParticipant => this.handleWowzaParticipantAdded(wowzaParticipant)); this.eventService.getAudioPaused().subscribe(async (message: AudioRecordingPauseStateMessage) => { if (this.conference.id === message.conferenceId) { @@ -77,28 +66,52 @@ export class AudioRecordingService { this.dialOutUUID = this.dialOutUUID.filter(uuid => uuid !== this.wowzaAgent.uuid); } - async reconnectToWowza(failedToConnectCallback: Function) { + async reconnectToWowza(callback: Function = null) { this.restartActioned = true; - this.cleanupDialOutConnections(); this.videoCallService.connectWowzaAgent(this.conference.audioRecordingIngestUrl, async dialOutToWowzaResponse => { if (dialOutToWowzaResponse.status === 'success') { - await this.eventService.sendAudioRecordingPaused(this.conference.id, false); + this.logger.debug(`${this.loggerPrefix} dial-out request successful`); } else { - failedToConnectCallback(); + this.restartActioned = false; + if (callback) { + callback(); + } else { + this.wowzaAgentConnection$.next(false); + } } }); } - cleanupDialOutConnections() { + cleanupDialOutConnections(): void { this.logger.debug(`${this.loggerPrefix} Cleaning up dial out connections, if any {dialOutUUID: ${this.dialOutUUID}}`); - this.dialOutUUID?.forEach(uuid => { - this.videoCallService.disconnectWowzaAgent(uuid); - }); + + if (this.dialOutUUID.length > 0) { + this.dialOutUUID?.forEach(uuid => { + if (uuid) { + this.videoCallService.disconnectWowzaAgent(uuid); + } + }); + } + this.dialOutUUID = []; } - cleanupSubscriptions() { + cleanupSubscriptions(): void { this.onDestroy$.next(); this.onDestroy$.complete(); } + + private async handleWowzaParticipantAdded(participant: VHPexipParticipant) { + if (participant) { + this.dialOutUUID = [...new Set([...this.dialOutUUID, participant?.uuid])]; + } + this.wowzaAgent = participant; + if (participant?.isAudioOnlyCall) { + this.wowzaAgentConnection$.next(true); + this.restartActioned = false; + await this.eventService.sendAudioRecordingPaused(this.conference.id, false); + } else { + this.wowzaAgentConnection$.next(false); + } + } } diff --git a/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.spec.ts b/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.spec.ts index 61f357bcb1..1e13f0c05d 100644 --- a/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.spec.ts +++ b/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.spec.ts @@ -281,10 +281,6 @@ describe('JudgeWaitingRoomComponent when conference exists', () => { if (component.callbackTimeout) { clearTimeout(component.callbackTimeout); } - - if (component.audioRecordingInterval) { - clearInterval(component.callbackTimeout); - } }); const pexipParticipant: PexipParticipant = { @@ -545,51 +541,6 @@ describe('JudgeWaitingRoomComponent when conference exists', () => { }); }); - describe('verifyAudioRecordingStream', () => { - const currentConferenceRecordingInSessionForSeconds = 10; - const currentAudioRecordingStreamCheckIntervalSeconds = 30; - - beforeEach(() => { - component.continueWithNoRecording = false; - component.recordingSessionSeconds = currentConferenceRecordingInSessionForSeconds; - component.audioStreamIntervalSeconds = currentAudioRecordingStreamCheckIntervalSeconds; - }); - - it('should init audio recording interval', () => { - spyOn(component, 'verifyAudioRecordingStream'); - component.initAudioRecordingInterval(); - expect(component.audioRecordingInterval).toBeDefined(); - }); - - it('should accumulate when conference is in session', async () => { - component.conference.status = ConferenceStatus.InSession; - await component.verifyAudioRecordingStream(); - expect(component.recordingSessionSeconds).toBe( - currentConferenceRecordingInSessionForSeconds + currentAudioRecordingStreamCheckIntervalSeconds - ); - }); - - it('should reset when conference is not in session', async () => { - component.conference.status = ConferenceStatus.Paused; - await component.verifyAudioRecordingStream(); - expect(component.recordingSessionSeconds).toBe(0); - }); - - it('should switch the continueWithNoRecording flag to false when conference is not in session', async () => { - component.conference.status = ConferenceStatus.Paused; - component.continueWithNoRecording = true; - await component.verifyAudioRecordingStream(); - expect(component.continueWithNoRecording).toBe(false); - }); - - it('should not switch the continueWithNoRecording flag when conference is in session', async () => { - component.conference.status = ConferenceStatus.InSession; - component.continueWithNoRecording = true; - await component.verifyAudioRecordingStream(); - expect(component.continueWithNoRecording).toBe(true); - }); - }); - describe('shouldCurrentUserJoinHearing', () => { it('should return false if user is a host and status is not InHearing', () => { component.participant.status = ParticipantStatus.Available; @@ -937,11 +888,11 @@ describe('JudgeWaitingRoomComponent when conference exists', () => { eventsService.sendAudioRestartActioned.calls.reset(); }); - it('should continue with no recording when judge dismisses the audio recording alert mid hearing', async () => { + it('should continue with no recording when judge dismisses the audio recording alert mid hearing', () => { audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; - component.recordingSessionSeconds = 61; component.conference.status = ConferenceStatus.InSession; - await component.verifyAudioRecordingStream(); + + component.verifyAudioRecordingStream(); component.audioRestartCallback(true); @@ -949,60 +900,36 @@ describe('JudgeWaitingRoomComponent when conference exists', () => { expect(component.continueWithNoRecording).toBeTruthy(); }); - it('should only display one toast for audio recording issues', async () => { + it('should only display one toast for audio recording issues', () => { audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; - component.recordingSessionSeconds = 61; component.conference.status = ConferenceStatus.InSession; - await component.verifyAudioRecordingStream(); + + component.verifyAudioRecordingStream(); + expect(component.audioErrorRetryToast).toBeTruthy(); expect(notificationToastrService.showAudioRecordingErrorWithRestart).toHaveBeenCalledTimes(1); }); - it('should display audio recording alert when wowza agent not assigned', async () => { + it('should display audio recording alert when wowza agent not assigned', () => { audioRecordingServiceSpy.wowzaAgent = null; - component.recordingSessionSeconds = 61; + component.conference.status = ConferenceStatus.InSession; - await component.verifyAudioRecordingStream(); + + component.verifyAudioRecordingStream(); + expect(component.audioErrorRetryToast).toBeTruthy(); expect(notificationToastrService.showAudioRecordingErrorWithRestart).toHaveBeenCalled(); }); - it('should not display audio recording alert before 20 seconds has passed', async () => { - audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; - component.recordingSessionSeconds = 0; - component.conference.status = ConferenceStatus.InSession; - await component.verifyAudioRecordingStream(); - expect(component.audioErrorRetryToast).toBeFalsy(); - expect(notificationToastrService.showAudioRecordingErrorWithRestart).not.toHaveBeenCalled(); - }); - it('should not preform audio recording check if continuing with no recording', async () => { audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; - component.recordingSessionSeconds = 100; component.conference.status = ConferenceStatus.InSession; component.continueWithNoRecording = true; - await component.verifyAudioRecordingStream(); - expect(component.audioErrorRetryToast).toBeFalsy(); - expect(notificationToastrService.showAudioRecordingErrorWithRestart).not.toHaveBeenCalled(); - }); - it('should not preform audio recording check if hearing isnt InSession', async () => { - audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; - component.recordingSessionSeconds = 100; - component.conference.status = ConferenceStatus.Paused; - await component.verifyAudioRecordingStream(); - expect(component.audioErrorRetryToast).toBeFalsy(); - expect(notificationToastrService.showAudioRecordingErrorWithRestart).toHaveBeenCalledTimes(0); - }); + component.verifyAudioRecordingStream(); - it('should reset notification state if hearing status not InSession', async () => { - audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; - component.recordingSessionSeconds = 100; - component.conference.status = ConferenceStatus.Paused; - component.continueWithNoRecording = true; - await component.verifyAudioRecordingStream(); - expect(component.continueWithNoRecording).toBeFalsy(); expect(component.audioErrorRetryToast).toBeFalsy(); + expect(notificationToastrService.showAudioRecordingErrorWithRestart).not.toHaveBeenCalled(); }); it('when wowza listener missing, but toast for restart already open, do nothing', async () => { @@ -1015,27 +942,27 @@ describe('JudgeWaitingRoomComponent when conference exists', () => { expect(notificationToastrService.showAudioRecordingErrorWithRestart).toHaveBeenCalledTimes(0); }); - it('when audio stream triggered again before action, but toast for restart already open, do nothing', async () => { + it('when audio stream triggered again before action, but toast for restart already open, do nothing', () => { audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; component.continueWithNoRecording = false; component.audioErrorRetryToast = jasmine.createSpyObj(VhToastComponent, ['actioned']); - component.recordingSessionSeconds = 61; component.conference.status = ConferenceStatus.InSession; - await component.verifyAudioRecordingStream(); + + component.verifyAudioRecordingStream(); + expect(notificationToastrService.showAudioRecordingRestartFailure).toHaveBeenCalledTimes(0); expect(notificationToastrService.showAudioRecordingErrorWithRestart).toHaveBeenCalledTimes(0); }); describe('Multiple Hosts Audio Alert', () => { - it('When Audio Recording Alert is displayed, restart is actioned by another user and closed for the current user', async () => { + it('When Audio Recording Alert is displayed, restart is actioned by another user and closed for the current user', () => { // Arrange audioRecordingServiceSpy.wowzaAgent.isAudioOnlyCall = false; component.continueWithNoRecording = false; component.audioErrorRetryToast = null; - component.recordingSessionSeconds = 61; component.conference.status = ConferenceStatus.InSession; // Act - await component.verifyAudioRecordingStream(); + component.verifyAudioRecordingStream(); // Assert expect(notificationToastrService.showAudioRecordingErrorWithRestart).toHaveBeenCalled(); expect(component.audioErrorRetryToast).toBeTruthy(); diff --git a/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.ts b/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.ts index f4abd75af6..43552ffc2c 100644 --- a/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.ts +++ b/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/judge-waiting-room/judge-waiting-room.component.ts @@ -41,6 +41,7 @@ import { Store } from '@ngrx/store'; import { ConferenceState } from '../store/reducers/conference.reducer'; import { LaunchDarklyService } from '../../services/launch-darkly.service'; import { AudioRecordingService } from '../../services/audio-recording.service'; +import { getCountdownComplete } from '../store/selectors/conference.selectors'; @Component({ selector: 'app-judge-waiting-room', @@ -48,10 +49,7 @@ import { AudioRecordingService } from '../../services/audio-recording.service'; styleUrls: ['./judge-waiting-room.component.scss', '../waiting-room-global-styles.scss'] }) export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implements OnDestroy, OnInit { - audioRecordingInterval: NodeJS.Timer; continueWithNoRecording = false; - audioStreamIntervalSeconds = 10; - recordingSessionSeconds = 0; expanedPanel = true; hostWantsToJoinHearing = false; displayConfirmStartHearingPopup: boolean; @@ -375,34 +373,18 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen this.hostWantsToJoinHearing = false; } - initAudioRecordingInterval() { - this.audioRecordingInterval = setInterval(async () => { - await this.verifyAudioRecordingStream(); - }, this.audioStreamIntervalSeconds * 1000); - } - audioRestartCallback(continueWithNoRecording: boolean) { this.continueWithNoRecording = continueWithNoRecording; this.audioErrorRetryToast = null; } - async verifyAudioRecordingStream(): Promise { - if (this.conference.status === ConferenceStatus.InSession) { - this.logger.debug(`${this.loggerPrefixJudge} Recording Session Seconds: ${this.recordingSessionSeconds}`); - this.recordingSessionSeconds += this.audioStreamIntervalSeconds; - } else { - this.recordingSessionSeconds = 0; - this.continueWithNoRecording = false; - } - - // If the recording session is greater than 20 seconds, + verifyAudioRecordingStream() { // has not been set to continue without recording, - // conference is in-session, + // video is open, // the alert isn't open already, // Recording is not paused // and the audio streaming agent cannot be validated, then show the alert if ( - this.recordingSessionSeconds > 20 && !this.continueWithNoRecording && this.showVideo && !this.audioErrorRetryToast && @@ -431,11 +413,11 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen this.unreadMessageCount = count; } - leaveConsultation() { + async leaveConsultation() { if (this.isPrivateConsultation) { this.showLeaveConsultationModal(); } else { - this.leaveJudicialConsultation(); + await this.leaveJudicialConsultation(); } } @@ -538,9 +520,6 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen this.subscribeToClock(); this.startEventHubSubscribers(); this.connectToPexip(); - if (this.conference.audio_recording_required) { - this.initAudioRecordingInterval(); - } }); } catch (error) { this.logger.error(`${this.loggerPrefixJudge} Failed to initialise the judge waiting room`, error); @@ -558,24 +537,30 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen } }); - this.eventService.getHearingStatusMessage().subscribe(message => { - this.handleHearingStatusMessage(message); - }); + this.eventService + .getHearingStatusMessage() + .pipe(takeUntil(this.onDestroy$)) + .subscribe((message: ConferenceStatusMessage) => this.handleHearingStatusMessage(message)); this.audioRecordingService .getAudioRecordingPauseState() .pipe(takeUntil(this.onDestroy$)) - .subscribe((recordingPaused: boolean) => { - this.recordingPaused = recordingPaused; - if (!this.recordingPaused) { - this.initAudioRecordingInterval(); - } - }); + .subscribe((recordingPaused: boolean) => (this.recordingPaused = recordingPaused)); this.audioRecordingService .getWowzaAgentConnectionState() .pipe(takeUntil(this.onDestroy$)) - .subscribe(stateIsConnected => (stateIsConnected ? this.onWowzaConnected() : this.onWowzaDisconnected())); + .subscribe((stateIsConnected: boolean) => (stateIsConnected ? this.onWowzaConnected() : this.onWowzaDisconnected())); + + this.store + .select(getCountdownComplete) + .pipe(takeUntil(this.onDestroy$)) + .subscribe(complete => { + if (complete) { + this.logger.debug(`${this.loggerPrefixJudge} Hearing countdown complete`); + this.verifyAudioRecordingStream(); + } + }); } private onShouldReload(): void { @@ -634,11 +619,6 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen } private cleanUp() { - this.logger.debug(`${this.loggerPrefixJudge} Clearing intervals and subscriptions for JOH waiting room`, { - conference: this.conference?.id - }); - - clearInterval(this.audioRecordingInterval); this.cleanupVideoControlCacheLogic(); this.executeWaitingRoomCleanup(); this.audioRecordingService.cleanupSubscriptions(); @@ -652,15 +632,13 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen } private onWowzaDisconnected() { - if (this.audioRecordingService.restartActioned) { - this.notificationToastrService.showAudioRecordingRestartFailure(this.audioRestartCallback.bind(this)); - } else if ( - this.conference.audio_recording_required && - this.conference.status === ConferenceStatus.InSession && - !this.recordingPaused - ) { - this.logWowzaAlert(); - this.showAudioRecordingRestartAlert(); + if (this.conference.audio_recording_required && this.conference.status === ConferenceStatus.InSession && !this.recordingPaused) { + if (this.audioRecordingService.restartActioned) { + this.notificationToastrService.showAudioRecordingRestartFailure(this.audioRestartCallback.bind(this)); + } else { + this.logWowzaAlert(); + this.showAudioRecordingRestartAlert(); + } } } @@ -680,13 +658,13 @@ export class JudgeWaitingRoomComponent extends WaitingRoomBaseDirective implemen if (this.audioErrorRetryToast) { return; } - this.recordingSessionSeconds = 0; - clearInterval(this.audioRecordingInterval); - const failedToReconnectCallback = () => { + this.audioErrorRetryToast = this.notificationToastrService.showAudioRecordingErrorWithRestart(this.reconnectWowzaAgent); + } + + private reconnectWowzaAgent(): void { + this.audioRecordingService.cleanupDialOutConnections(); + this.audioRecordingService.reconnectToWowza.bind(this, () => { this.notificationToastrService.showAudioRecordingRestartFailure(this.audioRestartCallback.bind(this)); - }; - this.audioErrorRetryToast = this.notificationToastrService.showAudioRecordingErrorWithRestart( - this.audioRecordingService.reconnectToWowza.bind(this, failedToReconnectCallback) - ); + }); } } diff --git a/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/private-consultation-room-controls/private-consultation-room-controls.component.html b/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/private-consultation-room-controls/private-consultation-room-controls.component.html index bb80fb12cf..79e412ac19 100644 --- a/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/private-consultation-room-controls/private-consultation-room-controls.component.html +++ b/VideoWeb/VideoWeb/ClientApp/src/app/waiting-space/private-consultation-room-controls/private-consultation-room-controls.component.html @@ -647,7 +647,7 @@