adding isSip function to room #2427
Conversation
There was a problem hiding this comment.
Hi @chagai95, thank you for your contribution 👍
Please have a look at the checks, fix the issues and then re-request the view.
If you have any questions just ask them here in the comments.
| public isSip(): boolean { | ||
| for (const member of this.getMembers()) { | ||
| if(member.userId != this.client.getUserId()){ | ||
| if(member.userId.startsWith("@_sip_")){ |
There was a problem hiding this comment.
Where is this @_sip_ prefix specced?
There was a problem hiding this comment.
I don't think it's specced, how would you purpose we go about this?
Our users are all created with this prefix as per the config on the sip bridge, since it's not possible to change a mxid I think it's reasonable to hard code this for now.
https://gitlab.matrix.org/new-vector/voip/matrix-sip-bridge/-/blob/master/config_sample.yaml#L27
Do you perhaps have a better suggestion? 🙈
There was a problem hiding this comment.
That's specific to your config on that sip bridge, other sip bridges may have different config. SIP is also not a concept mentioned in the Matrix spec at all. This really cannot land in its current state, it'd be better done as part of the extensible profiles project matrix-org/matrix-spec-proposals#3795
A config is also just that configurable - so the prefix can change and the js-sdk would then do the wrong thing. Also what stops someone registering @_sip_real_user:matrix.org where now this would wrongly return true?
There was a problem hiding this comment.
Ok thx! Can you maybe elaborate on how this is related to extensible profiles? We would like to identify if a room is a sip room or not, couldn't we spec that as a room type or something?
There was a problem hiding this comment.
That is an alternative, though keep in mind room types are immutable, this function currently can change depending on the members in a room. Extensible profiles could make the sip bridge users have an is_sip flag
There was a problem hiding this comment.
If it was just a bit of dead code probably not, if it was something more generic to hide rooms with types the app doesnt know then probably
There was a problem hiding this comment.
Ok thanks! It's about creating a call list out of the sip rooms.
There was a problem hiding this comment.
@t3chguy I hope it's ok for me to ping you, I was just wondering why not use m.bridge? I've seen that used in Beeper...
There was a problem hiding this comment.
Just because one vendor uses it, doesn't make it specced. This is especially bad given the m. prefix which is reserved for specced things only
Signed-off-by: Chagai Friedlander [email protected]
Here's what your changelog entry will look like:
✨ Features