-
Notifications
You must be signed in to change notification settings - Fork 739
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
Native lookup fixes #6101
Native lookup fixes #6101
Conversation
…nd create a dm with the native user instead of with the sip user
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.
(can you check the compilation issues please?) |
Couldn't figure out where the actual errors are... can you help me with that? |
} else { | ||
// do the same if there is no corresponding native user. | ||
directRoomHelper.ensureDMExists(sipUserId) | ||
var nativeRoomId = session.getExistingDirectRoomWithUser(nativeUserId) |
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.
You are maybe not using the latest develop from mainline?
The code should be session .roomService() .getExistingDirectRoomWithUser(nativeUserId) now
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.
oops, completely missed that - copy pasted that part of the change from our fork, sorry.
// just create a DM with the native user | ||
nativeRoomId = directRoomHelper.ensureDMExists(nativeUserId) | ||
} | ||
Timber.d("lookupPhoneNumber with nativeUserId: $nativeUserId and nativeRoomId: $nativeRoomId") |
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 import of Timber
is missing.
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.
Yep, sorry 🙈
(it's confirmed I am not the only one to create PR without even having compiled the code locally!) |
I tried that, but still couldn't really figure out where the errors are... sorry... also sorry for not compiling locally 🙈 |
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.
The (pre-existing) variable names in here are quite confusing, but the logic of the change looks correct.
I think everything passed?
|
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.
The error reported by the CI can be ignored; we will try to fix that.
Thanks for the update, and thanks @dbkr for the review!
Type of change
Content
refactor - better naming, return native user id and not sip user id and create a dm with the native user instead of with the sip user
Motivation and context
When dialing an extension which has a native user behind it, a native matrix call happens instead of a matrix sip call because the user id of the sip user was used instead of the native one - so the sip virtual lookup (which comes in this case after the native lookup) failed. Otherwise also creating a native room when one does not exist seems like the right thing to do and the code looks more clear with these names.
Screenshots / GIFs
Tests
Tested devices
Checklist
Signed-off-by: Chagai Friedlander @me:chagai.website