-
Notifications
You must be signed in to change notification settings - Fork 731
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
Update Account Data with user matrix id for invited user by email #3743
Conversation
When an user has been invited by email to a DM, account data entry was stuck on the user email after the user account creation. When the user has joined element, an event m.room.member is triggered for each room attached to the user, containing a third party invite with the user matrix id. We use this event to update the user account with the matrix id.
9c98100
to
ac56b1e
Compare
} | ||
|
||
// remove roomId from currentDirectUserId entry | ||
hasUpdate = hasUpdate or(directChats[currentDirectUserId]?.remove(roomId) == true) |
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.
is it necessary to assign hasUpdate
here?
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.
Yes, the remove
method will modify the direct chat dictionary if the given item has been found and removed, so I want to edit this flag in that case.
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.
Yes, but I was wondering if hasUpdate
was not already equal to true in this case. It can maybe add some cleanup on some existing account data.
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.
Ok, I think I understand 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.
I'm not sure to understand, I use the or
function which will always call the second part of the operation (whether hasUpdate is true or not). The flag should be updated for each modification in the direct chats dictionary. In our case, the cleanup will be performed only for members related to the given directUserIdsToUpdate
dictionary.
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.
Some small remarks/question. Can you add a file for towncrier please?
...java/org/matrix/android/sdk/internal/session/sync/model/accountdata/DirectMessagesContent.kt
Outdated
Show resolved
Hide resolved
// remove roomId from currentDirectUserId entry | ||
hasUpdate = hasUpdate or(directChats[currentDirectUserId]?.remove(roomId) == true) | ||
// remove currentDirectUserId entry if there is no attached room anymore | ||
hasUpdate = hasUpdate or(directChats.takeIf { it[currentDirectUserId].isNullOrEmpty() }?.remove(currentDirectUserId) != null) |
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.
Same remark
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.
Same answer, If I remove a key from the directChats dictionary, I also want to edit this flag in order to update the accountData with the updated dictionary
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.
!= null
could be replaced by == true
, no?
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.
Nop, in that case, remove returns the removed object, if any or null otherwise
Done |
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.
LGTM, thanks for the update
Fixes #3643
Update the Account Data when an user has been invited to a DM by email (which is not bound to an existing account).
The DM is initially populated with the invited user email and stuck with this email when the user has joined Element by creating an account. Therefore, the
RoomSummary.directUserId
is also stuck on the email and does not match the otherUserId which has been updated with the user matrix id.To handle the user registration, we catch the
m.room.member
event which contains the matrix id as part of the third_party_invite content. Then, we check the account data dictionary and update the entries matching the roomId attached to the event, and where the user id does not match the matrix id pattern.Update the account data will then cause the update of the directUserId in the RoomSummary.