Skip to content

Commit b80c2f7

Browse files
committed
timeline: use the previous content's membership info when it's missing from the current membership event
Synapse returns a bare `{ "membership": "leave" }` as the content of a room membership event (for leave membership changes and likely others). In this case, it'd still be nice to have some kind of display name/avatar URL to show in UIs; it's possible to reuse information from the previous member event, if available.
1 parent 07b6425 commit b80c2f7

File tree

3 files changed

+89
-13
lines changed

3 files changed

+89
-13
lines changed

bindings/matrix-sdk-ffi/src/timeline/content.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@ use std::{collections::HashMap, sync::Arc};
1616

1717
use matrix_sdk::{crypto::types::events::UtdCause, room::power_levels::power_level_user_changes};
1818
use matrix_sdk_ui::timeline::{PollResult, TimelineDetails};
19-
use ruma::events::{
20-
room::{message::RoomMessageEventContentWithoutRelation, MediaSource},
21-
FullStateEventContent,
22-
};
19+
use ruma::events::room::{message::RoomMessageEventContentWithoutRelation, MediaSource};
2320
use tracing::warn;
2421

2522
use super::ProfileDetails;
@@ -35,7 +32,9 @@ impl TimelineItemContent {
3532

3633
match &self.0 {
3734
Content::Message(_) => TimelineItemContentKind::Message,
35+
3836
Content::RedactedMessage => TimelineItemContentKind::RedactedMessage,
37+
3938
Content::Sticker(sticker) => {
4039
let content = sticker.content();
4140
TimelineItemContentKind::Sticker {
@@ -44,23 +43,23 @@ impl TimelineItemContent {
4443
source: Arc::new(MediaSource::from(content.source.clone())),
4544
}
4645
}
46+
4747
Content::Poll(poll_state) => TimelineItemContentKind::from(poll_state.results()),
48+
4849
Content::CallInvite => TimelineItemContentKind::CallInvite,
50+
4951
Content::CallNotify => TimelineItemContentKind::CallNotify,
52+
5053
Content::UnableToDecrypt(msg) => {
5154
TimelineItemContentKind::UnableToDecrypt { msg: EncryptedMessage::new(msg) }
5255
}
56+
5357
Content::MembershipChange(membership) => TimelineItemContentKind::RoomMembership {
5458
user_id: membership.user_id().to_string(),
55-
user_display_name: if let FullStateEventContent::Original { content, .. } =
56-
membership.content()
57-
{
58-
content.displayname.clone()
59-
} else {
60-
None
61-
},
59+
user_display_name: membership.display_name(),
6260
change: membership.change().map(Into::into),
6361
},
62+
6463
Content::ProfileChange(profile) => {
6564
let (display_name, prev_display_name) = profile
6665
.displayname_change()
@@ -82,16 +81,19 @@ impl TimelineItemContent {
8281
prev_avatar_url: prev_avatar_url.flatten(),
8382
}
8483
}
84+
8585
Content::OtherState(state) => TimelineItemContentKind::State {
8686
state_key: state.state_key().to_owned(),
8787
content: state.content().into(),
8888
},
89+
8990
Content::FailedToParseMessageLike { event_type, error } => {
9091
TimelineItemContentKind::FailedToParseMessageLike {
9192
event_type: event_type.to_string(),
9293
error: error.to_string(),
9394
}
9495
}
96+
9597
Content::FailedToParseState { event_type, state_key, error } => {
9698
TimelineItemContentKind::FailedToParseState {
9799
event_type: event_type.to_string(),

crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,38 @@ impl RoomMembershipChange {
432432
&self.content
433433
}
434434

435+
/// Retrieve the member's display name from the current event, or, if
436+
/// missing, from the one it replaced.
437+
pub fn display_name(&self) -> Option<String> {
438+
if let FullStateEventContent::Original { content, prev_content } = &self.content {
439+
content
440+
.displayname
441+
.as_ref()
442+
.or_else(|| {
443+
prev_content.as_ref().and_then(|prev_content| prev_content.displayname.as_ref())
444+
})
445+
.cloned()
446+
} else {
447+
None
448+
}
449+
}
450+
451+
/// Retrieve the avatar URL from the current event, or, if missing, from the
452+
/// one it replaced.
453+
pub fn avatar_url(&self) -> Option<OwnedMxcUri> {
454+
if let FullStateEventContent::Original { content, prev_content } = &self.content {
455+
content
456+
.avatar_url
457+
.as_ref()
458+
.or_else(|| {
459+
prev_content.as_ref().and_then(|prev_content| prev_content.avatar_url.as_ref())
460+
})
461+
.cloned()
462+
} else {
463+
None
464+
}
465+
}
466+
435467
/// The membership change induced by this event.
436468
///
437469
/// If this returns `None`, it doesn't mean that there was no change, but

crates/matrix-sdk-ui/src/timeline/tests/basic.rs

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use ruma::{
3131
},
3232
FullStateEventContent,
3333
},
34-
owned_event_id, MilliSecondsSinceUnixEpoch,
34+
owned_event_id, owned_mxc_uri, MilliSecondsSinceUnixEpoch,
3535
};
3636
use stream_assert::assert_next_matches;
3737

@@ -191,7 +191,7 @@ async fn test_room_member() {
191191
.handle_live_state_event_with_state_key(
192192
&ALICE,
193193
ALICE.to_owned(),
194-
third_room_member_content,
194+
third_room_member_content.clone(),
195195
Some(second_room_member_content),
196196
)
197197
.await;
@@ -201,6 +201,48 @@ async fn test_room_member() {
201201
assert_matches!(profile.displayname_change(), Some(_));
202202
assert_matches!(profile.avatar_url_change(), None);
203203

204+
let mut fourth_room_member_content = RoomMemberEventContent::new(MembershipState::Join);
205+
fourth_room_member_content.displayname = Some("Alice In Wonderland".to_owned());
206+
fourth_room_member_content.avatar_url = Some(owned_mxc_uri!("mxc://lolcathost.io/abc"));
207+
timeline
208+
.handle_live_state_event_with_state_key(
209+
&ALICE,
210+
ALICE.to_owned(),
211+
fourth_room_member_content.clone(),
212+
Some(third_room_member_content),
213+
)
214+
.await;
215+
216+
let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value);
217+
assert_let!(TimelineItemContent::ProfileChange(profile) = item.content());
218+
assert_matches!(profile.displayname_change(), None);
219+
assert_matches!(profile.avatar_url_change(), Some(_));
220+
221+
{
222+
// No avatar or display name in the new room member event content, but it's
223+
// possible to get the previous one using the getters.
224+
let room_member_content = RoomMemberEventContent::new(MembershipState::Leave);
225+
226+
timeline
227+
.handle_live_state_event_with_state_key(
228+
&ALICE,
229+
ALICE.to_owned(),
230+
room_member_content,
231+
Some(fourth_room_member_content),
232+
)
233+
.await;
234+
235+
let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value);
236+
assert_let!(TimelineItemContent::MembershipChange(membership) = item.content());
237+
assert_matches!(membership.display_name().as_deref(), Some("Alice In Wonderland"));
238+
assert_matches!(
239+
membership.avatar_url().map(|url| url.to_string()).as_deref(),
240+
Some("mxc://lolcathost.io/abc")
241+
);
242+
assert_matches!(membership.content(), FullStateEventContent::Original { .. });
243+
assert_matches!(membership.change(), Some(MembershipChange::Left));
244+
}
245+
204246
timeline
205247
.handle_live_redacted_state_event_with_state_key(
206248
&ALICE,

0 commit comments

Comments
 (0)