(GH178) fix(audio): Persist enabled sounds state (twilio#179)

* test: change the test to reflect the reality * fix(audio): persist enabled sounds state when calling device.updateOptions * docs: update changelog * docs: make the enabled sounds getter as private --------- Co-authored-by: Kamal Bennani <[email protected]>
podium · Jul 5, 2023 · ec9b190 · ec9b190
1 parent f551b3b
commit ec9b190
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,8 @@ Bug Fixes
   });
   ```
 
+- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/178) where calling `device.updateOptions` would reset the `device.audio._enabledSounds` state.
+
 2.6.0 (June 20, 2023)
 =====================
 

diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts
@@ -185,6 +185,10 @@ class AudioHelper extends EventEmitter {
     const isAudioContextSupported: boolean = !!(options.AudioContext || options.audioContext);
     const isEnumerationSupported: boolean = !!this._enumerateDevices;
 
+    if (options.enabledSounds) {
+      this._enabledSounds = options.enabledSounds;
+    }
+
     const isSetSinkSupported: boolean = typeof options.setSinkId === 'function';
     this.isOutputSelectionSupported = isEnumerationSupported && isSetSinkSupported;
     this.isVolumeSupported = isAudioContextSupported;
@@ -234,6 +238,14 @@ class AudioHelper extends EventEmitter {
     }
   }
 
+  /**
+   * Current state of the enabled sounds
+   * @private
+   */
+  _getEnabledSounds(): Record<Device.ToggleableSound, boolean> {
+    return this._enabledSounds;
+  }
+
   /**
    * Start polling volume if it's supported and there's an input stream to poll.
    * @private
@@ -709,6 +721,11 @@ namespace AudioHelper {
      */
     audioContext?: AudioContext;
 
+    /**
+     * Whether each sound is enabled.
+     */
+    enabledSounds?: Record<Device.ToggleableSound, boolean>;
+
     /**
      * Overrides the native MediaDevices.enumerateDevices API.
      */

diff --git a/lib/twilio/device.ts b/lib/twilio/device.ts
@@ -1310,19 +1310,22 @@ class Device extends EventEmitter {
    * Set up an audio helper for usage by this {@link Device}.
    */
   private _setupAudioHelper(): void {
+    const audioOptions: AudioHelper.Options = {
+      audioContext: Device.audioContext,
+      enumerateDevices: this._options.enumerateDevices,
+    };
+
     if (this._audio) {
       this._log.info('Found existing audio helper; destroying...');
+      audioOptions.enabledSounds = this._audio._getEnabledSounds();
       this._destroyAudioHelper();
     }
 
     this._audio = new (this._options.AudioHelper || AudioHelper)(
       this._updateSinkIds,
       this._updateInputStream,
       this._options.getUserMedia || getUserMedia,
-      {
-        audioContext: Device.audioContext,
-        enumerateDevices: this._options.enumerateDevices,
-      },
+      audioOptions,
     );
 
     this._audio.on('deviceChange', (lostActiveDevices: MediaDeviceInfo[]) => {

diff --git a/tests/unit/device.ts b/tests/unit/device.ts
@@ -13,6 +13,7 @@ import * as assert from 'assert';
 import { EventEmitter } from 'events';
 import { SinonFakeTimers, SinonSpy, SinonStubbedInstance } from 'sinon';
 import * as sinon from 'sinon';
+import AudioHelper from '../../lib/twilio/audiohelper';
 
 const root = global as any;
 
@@ -38,10 +39,17 @@ describe('Device', function() {
 
   const sounds: Partial<Record<Device.SoundName, any>> = { };
 
-  const AudioHelper = (_updateSinkIds: Function, _updateInputStream: Function) => {
+  const AudioHelper = (_updateSinkIds: Function, _updateInputStream: Function, getUserMedia: Function, options?: AudioHelper.Options) => {
+    enabledSounds = options?.enabledSounds || {
+      [Device.SoundName.Disconnect]: true,
+      [Device.SoundName.Incoming]: true,
+      [Device.SoundName.Outgoing]: true,
+    };
     updateInputStream = _updateInputStream;
     updateSinkIds = _updateSinkIds;
     const audioHelper = createEmitterStub(require('../../lib/twilio/audiohelper').default);
+    audioHelper._enabledSounds = enabledSounds;
+    audioHelper._getEnabledSounds = () => enabledSounds;
     audioHelper.disconnect = () => enabledSounds[Device.SoundName.Disconnect];
     audioHelper.incoming = () => enabledSounds[Device.SoundName.Incoming];
     audioHelper.outgoing = () => enabledSounds[Device.SoundName.Outgoing];
@@ -68,11 +76,6 @@ describe('Device', function() {
   });
 
   beforeEach(() => {
-    enabledSounds = {
-      [Device.SoundName.Disconnect]: true,
-      [Device.SoundName.Incoming]: true,
-      [Device.SoundName.Outgoing]: true,
-    };
     pstream = null;
     publisher = null;
     clock = sinon.useFakeTimers(Date.now());
@@ -247,6 +250,20 @@ describe('Device', function() {
           activeCall.emit('accept');
           sinon.assert.notCalled(spy.play);
         });
+
+        it('should not play outgoing sound after accepted if disabled after calling updateOptions', async () => {
+          enabledSounds[Device.SoundName.Outgoing] = false;
+          // Force a new audio-helper instance to be recreated
+          // After updating the enabled sounds state
+          device.updateOptions();
+          const spy: any = { play: sinon.spy() };
+          device['_soundcache'].set(Device.SoundName.Outgoing, spy);
+          await device.connect();
+          activeCall._direction = 'OUTGOING';
+          activeCall.emit('accept');
+          // should fail
+          sinon.assert.notCalled(spy.play);
+        });
       });
 
       describe('.destroy()', () => {
-Original file line number
+Diff line change
@@ Expand Up / @@ -23,6 +23,8 @@ Bug Fixes @@
       });
       ```
+    - Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/178) where calling `device.updateOptions` would reset the `device.audio._enabledSounds` state.
 .6.0 (June 20, 2023)
     =====================
@@ Expand Down @@