fix(sdk): mark invited DM rooms joined with Client::join_room_by_id or Client::join_room_by_id_or_alias as DMs
#5043
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5043 +/- ##
==========================================
+ Coverage 85.83% 85.84% +0.01%
==========================================
Files 325 325
Lines 35896 35900 +4
==========================================
+ Hits 30810 30820 +10
+ Misses 5086 5080 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thank you, testing this on EXI it works well 👏
Linking: element-hq/element-x-ios#3958
crates/matrix-sdk/src/client/mod.rs
Outdated
|
|
||
| let mark_as_dm = self.must_mark_joined_room_as_dm(&response.room_id).await; | ||
| let base_room = self.base_client().room_joined(&response.room_id).await?; | ||
| Ok(Room::new(self.clone(), base_room)) | ||
| let room = Room::new(self.clone(), base_room); | ||
| if mark_as_dm { | ||
| room.set_is_direct(true).await?; | ||
| } | ||
|
|
||
| Ok(room) |
There was a problem hiding this comment.
Can't we put this code snippet into a function? Seems like we have the same thing in two places now, unless I'm missing something.
There was a problem hiding this comment.
You're right, I've put all of this inside must_mark_joined_room_as_dm and renamed it finish_join_room 👍
bnjbvr
force-pushed
the
bnjbvr/fix-joined-room-not-marked-as-dms
branch
from
05ce616 to
bb5a82b
Compare
…d_or_alias` as DMs if needs be This moves the logic from `Room::join()` to these two methods. This is isofunctional, because `Room::join()` does call into `Client::join_room_by_id()` internally.
bnjbvr
force-pushed
the
bnjbvr/fix-joined-room-not-marked-as-dms
branch
from
bb5a82b to
e833028
Compare
| /// in the DM event after creating the joined room. | ||
| async fn finish_join_room(&self, room_id: &RoomId) -> Result<Room> { | ||
| let mark_as_dm = if let Some(room) = self.get_room(room_id) { | ||
| room.state() == RoomState::Invited |
There was a problem hiding this comment.
won't this race with the /sync response, causing it to do the wrong thing if the /sync response arrives before the /join completes?
There was a problem hiding this comment.
Good catch 👍 I suppose passing the previous room state, in the callers, captured before sending the join_room requests, would be sufficient to avoid the race (but would also need some kind of regression test to prove it can happen in the first place).
This moves the logic from
Room::join()to these two methods. This is isofunctional, becauseRoom::join()does call intoClient::join_room_by_id()internally.Fix #4730.