-
Notifications
You must be signed in to change notification settings - Fork 24
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: prevent auto-dropping already accepted or rejected calls #1619
base: main
Are you sure you want to change the base?
Conversation
@@ -2000,28 +2016,33 @@ export class Call { | |||
* Applicable only for ringing calls. | |||
*/ | |||
private scheduleAutoDrop = () => { | |||
clearTimeout(this.dropTimeout); | |||
this.leaveCallHooks.add( | |||
createSubscription(this.state.settings$, (settings) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, a subscription on call settings was set up when scheduling auto-drop. I don't see a reason to set up the subscription here.
Call settings should be up-to-date at this point; and in case call settings were somehow update mid-ringing, this subscription didn't take updated timeouts into account correctly anyway.
So, to simplify things, we are just setting up a timeout using current call settings.
if ( | ||
callingState === CallingState.JOINED || | ||
callingState === CallingState.JOINING || | ||
callingState === CallingState.LEFT | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cancelling auto-drop on calling state change is no longer required, since it's covered by two other checks:
- Auto-drop is cancelled if the call is either accepted or rejected by the current user.
- Auto-drop is a no-op if calling state changed to anything other than
RINGING
.
this.dropTimeout = setTimeout(() => { | ||
// the call might have stopped ringing by this point, | ||
// e.g. it was already accepted and joined | ||
if (this.state.callingState !== CallingState.RINGING) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An additional guard to prevent dropping a call that is no longer RINGING
(e.g. was accepted or rejected on the current device/tab).
This PR compliments https://github.com/GetStream/chat/pull/7669 and prevents auto-rejection of a ringing call after it was accepted or rejected by the current user (on another device or in another tab).
Note that backend changes made in https://github.com/GetStream/chat/pull/7669 make this client update less critical, but it's still nice to avoid an extra API call.