From 61be6e8b787464047bd096da6a5454c05d5302c0 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 3 Jul 2024 17:03:34 +0200 Subject: [PATCH] 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. --- .../matrix-sdk-ffi/src/timeline/content.rs | 24 +++++----- .../src/timeline/event_item/content/mod.rs | 32 +++++++++++++ .../matrix-sdk-ui/src/timeline/tests/basic.rs | 46 ++++++++++++++++++- 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline/content.rs b/bindings/matrix-sdk-ffi/src/timeline/content.rs index 49e2544d03a..b9c92051c94 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/content.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/content.rs @@ -16,10 +16,7 @@ use std::{collections::HashMap, sync::Arc}; use matrix_sdk::{crypto::types::events::UtdCause, room::power_levels::power_level_user_changes}; use matrix_sdk_ui::timeline::{PollResult, TimelineDetails}; -use ruma::events::{ - room::{message::RoomMessageEventContentWithoutRelation, MediaSource}, - FullStateEventContent, -}; +use ruma::events::room::{message::RoomMessageEventContentWithoutRelation, MediaSource}; use tracing::warn; use super::ProfileDetails; @@ -35,7 +32,9 @@ impl TimelineItemContent { match &self.0 { Content::Message(_) => TimelineItemContentKind::Message, + Content::RedactedMessage => TimelineItemContentKind::RedactedMessage, + Content::Sticker(sticker) => { let content = sticker.content(); TimelineItemContentKind::Sticker { @@ -44,23 +43,23 @@ impl TimelineItemContent { source: Arc::new(MediaSource::from(content.source.clone())), } } + Content::Poll(poll_state) => TimelineItemContentKind::from(poll_state.results()), + Content::CallInvite => TimelineItemContentKind::CallInvite, + Content::CallNotify => TimelineItemContentKind::CallNotify, + Content::UnableToDecrypt(msg) => { TimelineItemContentKind::UnableToDecrypt { msg: EncryptedMessage::new(msg) } } + Content::MembershipChange(membership) => TimelineItemContentKind::RoomMembership { user_id: membership.user_id().to_string(), - user_display_name: if let FullStateEventContent::Original { content, .. } = - membership.content() - { - content.displayname.clone() - } else { - None - }, + user_display_name: membership.display_name(), change: membership.change().map(Into::into), }, + Content::ProfileChange(profile) => { let (display_name, prev_display_name) = profile .displayname_change() @@ -82,16 +81,19 @@ impl TimelineItemContent { prev_avatar_url: prev_avatar_url.flatten(), } } + Content::OtherState(state) => TimelineItemContentKind::State { state_key: state.state_key().to_owned(), content: state.content().into(), }, + Content::FailedToParseMessageLike { event_type, error } => { TimelineItemContentKind::FailedToParseMessageLike { event_type: event_type.to_string(), error: error.to_string(), } } + Content::FailedToParseState { event_type, state_key, error } => { TimelineItemContentKind::FailedToParseState { event_type: event_type.to_string(), diff --git a/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs b/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs index 5be2206337b..87e6e64d457 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_item/content/mod.rs @@ -432,6 +432,38 @@ impl RoomMembershipChange { &self.content } + /// Retrieve the member's display name from the current event, or, if + /// missing, from the one it replaced. + pub fn display_name(&self) -> Option { + if let FullStateEventContent::Original { content, prev_content } = &self.content { + content + .displayname + .as_ref() + .or_else(|| { + prev_content.as_ref().and_then(|prev_content| prev_content.displayname.as_ref()) + }) + .cloned() + } else { + None + } + } + + /// Retrieve the avatar URL from the current event, or, if missing, from the + /// one it replaced. + pub fn avatar_url(&self) -> Option { + if let FullStateEventContent::Original { content, prev_content } = &self.content { + content + .avatar_url + .as_ref() + .or_else(|| { + prev_content.as_ref().and_then(|prev_content| prev_content.avatar_url.as_ref()) + }) + .cloned() + } else { + None + } + } + /// The membership change induced by this event. /// /// If this returns `None`, it doesn't mean that there was no change, but diff --git a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs index 870eaad1a01..08878b6917b 100644 --- a/crates/matrix-sdk-ui/src/timeline/tests/basic.rs +++ b/crates/matrix-sdk-ui/src/timeline/tests/basic.rs @@ -31,7 +31,7 @@ use ruma::{ }, FullStateEventContent, }, - owned_event_id, MilliSecondsSinceUnixEpoch, + owned_event_id, owned_mxc_uri, MilliSecondsSinceUnixEpoch, }; use stream_assert::assert_next_matches; @@ -191,7 +191,7 @@ async fn test_room_member() { .handle_live_state_event_with_state_key( &ALICE, ALICE.to_owned(), - third_room_member_content, + third_room_member_content.clone(), Some(second_room_member_content), ) .await; @@ -201,6 +201,48 @@ async fn test_room_member() { assert_matches!(profile.displayname_change(), Some(_)); assert_matches!(profile.avatar_url_change(), None); + let mut fourth_room_member_content = RoomMemberEventContent::new(MembershipState::Join); + fourth_room_member_content.displayname = Some("Alice In Wonderland".to_owned()); + fourth_room_member_content.avatar_url = Some(owned_mxc_uri!("mxc://lolcathost.io/abc")); + timeline + .handle_live_state_event_with_state_key( + &ALICE, + ALICE.to_owned(), + fourth_room_member_content.clone(), + Some(third_room_member_content), + ) + .await; + + let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + assert_let!(TimelineItemContent::ProfileChange(profile) = item.content()); + assert_matches!(profile.displayname_change(), None); + assert_matches!(profile.avatar_url_change(), Some(_)); + + { + // No avatar or display name in the new room member event content, but it's + // possible to get the previous one using the getters. + let room_member_content = RoomMemberEventContent::new(MembershipState::Leave); + + timeline + .handle_live_state_event_with_state_key( + &ALICE, + ALICE.to_owned(), + room_member_content, + Some(fourth_room_member_content), + ) + .await; + + let item = assert_next_matches!(stream, VectorDiff::PushBack { value } => value); + assert_let!(TimelineItemContent::MembershipChange(membership) = item.content()); + assert_matches!(membership.display_name().as_deref(), Some("Alice In Wonderland")); + assert_matches!( + membership.avatar_url().map(|url| url.to_string()).as_deref(), + Some("mxc://lolcathost.io/abc") + ); + assert_matches!(membership.content(), FullStateEventContent::Original { .. }); + assert_matches!(membership.change(), Some(MembershipChange::Left)); + } + timeline .handle_live_redacted_state_event_with_state_key( &ALICE,