-
Notifications
You must be signed in to change notification settings - Fork 252
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
fix(sdk): when an encryption event is received, mark the room encryption as set #3602
fix(sdk): when an encryption event is received, mark the room encryption as set #3602
Conversation
… encryption as set. `RoomInfo::handle_state_event` will check this condition so we don't have to later ask the HS whether the room is encrypted or not through `room::is_encrypted()`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3602 +/- ##
==========================================
- Coverage 83.88% 83.88% -0.01%
==========================================
Files 254 254
Lines 26306 26309 +3
==========================================
Hits 22068 22068
- Misses 4238 4241 +3 ☔ View full report in Codecov by Sentry. |
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.
Looks good, left a nit about the documentation.
@@ -566,6 +566,10 @@ impl RoomListItem { | |||
self.inner.init_timeline_with_builder(timeline_builder).map_err(RoomListError::from).await | |||
} | |||
|
|||
async fn is_encrypted(&self) -> bool { |
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.
Please add a doc comment, documentation is nowadays visible over the FFI, also please document that the correctness of this might depend on specifying m.room.encryption
as required state.
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.
Let me know if 0162f6b works for you.
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.
RoomInfo::handle_state_event(...)
will check this condition so we don't have to later ask the HS whether the room is encrypted or not throughroom::is_encrypted()
.Also, as we need some way to get this info in the room list in our clients, I also added 2 ways of doing this the FFI crate, one through
RoomInfo
and another one throughRoomListItem
. If we don't want the duplication I can get rid of one of them.This bug was found by @poljar, thanks for pointing it out!
Signed-off-by: