Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bug): call._setPrimary should set _primary before _emit is sent #109

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

skyler-murray
Copy link

Description

Ran into this while testing out multiple call handling. This fixes a subtle bug where we set _primary after sending the event so the event and the call list didn't have a primary call set. This is due to the this._emit('promoted', this) being called before we set the call to primary and so all the data in the event has no primary call set.

My argument for doing this is that at this point the call has become the primary and should be reported as such with the event. The call.demoted logic follows the same pattern and sets this.primary = false before sending the demoted event.

Scenario:

  1. You have two active calls (A, B). One is on hold (B) while the primary (A) is either on hold or not (it doesn't matter).
  2. Call lwpCallList.switchCall(B.callId()) to make B the active call
  3. A gets put on hold
  4. B is promoted to primary and no longer on hold
  5. Received call.promoted event for B

Expected:
call.isPrimary() for B returns true
_lwp.getCallList() returns a list of calls [A, B] and B.isPrimary() === true

Actual
call.isPrimary() for B returns false
_lwp.getCallList() returns a list of calls [A, B] and B.isPrimary() === false

Fixed a subtle bug where we set _primary after sending the event so the event and the call list didn't have a primary call set.
@skyler-murray skyler-murray changed the title fix(bug): _setPrimary should set _primary before _emit is sent fix(bug): call._setPrimary should set _primary before _emit is sent Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant