feat(Room): Check if the user is allowed to do a room mention before trying to send a call notify event. #4271
Conversation
…trying to send a call notify event.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4271 +/- ##
==========================================
+ Coverage 85.00% 85.05% +0.04%
==========================================
Files 274 274
Lines 29945 29949 +4
==========================================
+ Hits 25456 25474 +18
+ Misses 4489 4475 -14 ☔ View full report in Codecov by Sentry. |
| if !self.can_user_trigger_room_notification(self.own_user_id()).await? { | ||
| return Ok(()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you add a description on the PR as to why this change is needed and more importantly why not still send the call notification but without Mentions::with_room_mention?
There was a problem hiding this comment.
At the moment we just do not need it. All clients would just ignore a non mention notify event.
But your question makes me think. We could say we still want to send it.
We do use the notify event for the timeline rendering atm. But this really is not what it is supposed to be used for.
Those are really ephemeral events (but we do not have custom ephemeral events).
So sending them if no client reacts to them is just unnecassary.
The question is if we might want to introduce more complexity and allow ringing in rooms that do have all messages notification settings to still ring even though a user sends a "non mention" ring event...
Which is sth I like:
- A non mention notify will only ring for users that set the room to "all messages"
- A mention notify will ring for all mentioned users.
So both sides have some control.
There was a problem hiding this comment.
So basically we need a lot of UX discussions (also about when to ring and when to just notify)
This is an okay addition to what we have now before we converge on a general behaviour we want for all combinations of this:
- ring, notify
- mention, non mention
Signed-off-by: