-
Notifications
You must be signed in to change notification settings - Fork 332
bindings: MSC4310 call decline and subscribe to decline events #5614
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5614 +/- ##
=======================================
Coverage 88.89% 88.89%
=======================================
Files 345 346 +1
Lines 95800 95840 +40
Branches 95800 95840 +40
=======================================
+ Hits 85158 85200 +42
+ Misses 6655 6651 -4
- Partials 3987 3989 +2 ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #5614 will not alter performanceComparing Summary
|
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.
Looking mostly good, I left some nits here and there.
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.
Thanks, looks good. Let's just add a changelog entry for this.
Added f70afad |
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.
Thanks, looks good.
// The event must be CallNotify-like. | ||
if let AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::CallNotify(notify)) = event { | ||
if notify.sender() == own_user_id { | ||
// Cannot decline own call. | ||
Err(CallError::DeclineOwnCall) | ||
} else { | ||
Ok(RtcDeclineEventContent::new(notification_event_id)) | ||
} | ||
} else { | ||
Err(CallError::BadEventType) | ||
} |
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.
EC will always sent both events (...rtc.notifcation
+ ...call.notify
) but the spec does not define a relation to a call.notify event (they are deprecated).
To make all platforms consistent, we would need to decline on the rtc.notification event. Otherwise EW will not detect the decline and will not stop ringing.
As a stop gap we could patch EW to listen to any decline event independent of its m.relation.event_id
.
But this is only really correct if it checks for m.rtc notification introduced here: ruma/ruma#2199
Fixes #5580
Signed-off-by: